https://github.com/KHicketts updated https://github.com/llvm/llvm-project/pull/196272
>From 52d11c1082709188e7fa4d8b6995febf1bcf19fa Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Tue, 28 Apr 2026 14:37:52 +0100 Subject: [PATCH 01/12] add sema changes for class attribute --- clang/lib/Sema/SemaDeclAttr.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index e47a30193567f..243c66b474384 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6462,6 +6462,29 @@ static void handleAbiTagAttr(Sema &S, Decl *D, const ParsedAttr &AL) { AbiTagAttr(S.Context, AL, Tags.data(), Tags.size())); } +// for now this only handles std::optional (POC) +static bool isValidAnalyseAsClassAttr(Decl *D, StringRef Tag) { + if (Tag == "std::optional") + return true; + return false; +} + +static void handleAnalyseAsClass(Sema &S, Decl *D, const ParsedAttr &AL) { + StringRef Str; + if (!S.checkStringLiteralArgumentAttr(AL, 0, Str)) + return; + if (D->hasAttr<AnalyseAsClassAttr>()) { + S.Diag(AL.getLoc(), diag::err_duplicate_attribute) << AL; + return; + } + if (!isValidAnalyseAsClassAttr(D, Str)) { + S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL; + return; + } + + D->addAttr(::new (S.Context) AnalyseAsClassAttr(S.Context, AL, Str)); +} + static bool hasBTFDeclTagAttr(Decl *D, StringRef Tag) { for (const auto *I : D->specific_attrs<BTFDeclTagAttr>()) { if (I->getBTFDeclTag() == Tag) @@ -7551,6 +7574,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_BPFPreserveStaticOffset: handleSimpleAttribute<BPFPreserveStaticOffsetAttr>(S, D, AL); break; + case ParsedAttr::AT_AnalyseAsClass: + handleAnalyseAsClass(S, D, AL); + break; case ParsedAttr::AT_BTFDeclTag: handleBTFDeclTagAttr(S, D, AL); break; >From fe028c7292be79ba6f753ebae1039e65aa0032e9 Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Tue, 28 Apr 2026 15:43:52 +0100 Subject: [PATCH 02/12] analyse_as_class works ok --- .../Models/UncheckedOptionalAccessModel.cpp | 4 + hicketts_test/hicketts_optional.h | 83 +++++++++++++++++++ hicketts_test/test_hicketts_optional.cpp | 83 +++++++++++++++++++ 3 files changed, 170 insertions(+) create mode 100644 hicketts_test/hicketts_optional.h create mode 100644 hicketts_test/test_hicketts_optional.cpp diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 568564fb361f4..317f5034b0145 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h" +#include "clang/AST/Attr.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" @@ -89,6 +90,9 @@ static bool hasOptionalClassName(const CXXRecordDecl &RD) { isFullyQualifiedNamespaceEqualTo(*N, "bdlb", "BloombergLP"); } + if (RD.hasAttr<AnalyseAsClassAttr>()) + return true; + return false; } diff --git a/hicketts_test/hicketts_optional.h b/hicketts_test/hicketts_optional.h new file mode 100644 index 0000000000000..b915b01f70290 --- /dev/null +++ b/hicketts_test/hicketts_optional.h @@ -0,0 +1,83 @@ +#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::analyse_as_class("std::optional")]] HickettsOptional { + T *storage_ = nullptr; + +public: + constexpr HickettsOptional() noexcept {} + + constexpr HickettsOptional(nothing_t) noexcept {} + + HickettsOptional(const HickettsOptional &) = default; + + HickettsOptional(HickettsOptional &&) = default; + + // Equivalent to std::optional::value() + //[[clang_analyse_as_method(std::optional::value)]] + const T &unwrap() const & { return *storage_; } + T &unwrap() & { return *storage_; } + const T &&unwrap() const && { return static_cast<const T &&>(*storage_); } + 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*() + const T &deref() const & { return *storage_; } + 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 / has_value() + constexpr bool hasValue() const noexcept { return storage_ != nullptr; } + constexpr explicit operator bool() const noexcept { return storage_ != nullptr; } + constexpr bool isPresent() const noexcept { return storage_ != nullptr; } + constexpr bool isEmpty() 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> + T &construct(Args &&...args) { return *storage_; } + + // Equivalent to std::optional::reset() + void clear() noexcept { storage_ = nullptr; } + + // Equivalent to std::optional::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; } +}; + +} // namespace mylib + +#endif // HICKETTS_OPTIONAL_H_ diff --git a/hicketts_test/test_hicketts_optional.cpp b/hicketts_test/test_hicketts_optional.cpp new file mode 100644 index 0000000000000..fa0153a79f341 --- /dev/null +++ b/hicketts_test/test_hicketts_optional.cpp @@ -0,0 +1,83 @@ +// Test cases for mylib::HickettsOptional — a custom optional-like type +// with differently named functions. +// +// Run from hicketts_test/ with: +// clang-tidy -checks='bugprone-unchecked-optional-access' \ +// test_hicketts_optional.cpp -- -I . -Wno-undefined-inline + +#include "hicketts_optional.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.hasValue()) { + 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() + } +} + +// --- State changes --- + +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 + } +} + +// --- 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; +} >From ec4ea5a3de75bcc71b0140a11f24298d4681fdb0 Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Tue, 28 Apr 2026 16:35:01 +0100 Subject: [PATCH 03/12] partially working for per method analysis --- .../Models/UncheckedOptionalAccessModel.cpp | 19 +++++++++++-- clang/lib/Sema/SemaDeclAttr.cpp | 27 +++++++++++++++++++ hicketts_test/hicketts_optional.h | 17 ++++++------ 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 317f5034b0145..f2c9aa756943e 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -230,6 +230,20 @@ AST_MATCHER(CXXOperatorCallExpr, hasOptionalOperatorObjectType) { return hasReceiverTypeDesugaringToOptional(Node.getArg(0)); } +AST_MATCHER_P(NamedDecl, hasAnalyseAsMethodName, std::string, MethodName) { + if (const auto *MD = dyn_cast<CXXMethodDecl>(&Node)) { + if (const auto *Attr = MD->getAttr<AnalyseAsMethodAttr>()) { + StringRef AttrValue = Attr->getMethodName(); + auto Pos = AttrValue.rfind("::"); + StringRef AttrMethodName = (Pos != StringRef::npos) + ? AttrValue.substr(Pos + 2) + : AttrValue; + return AttrMethodName == MethodName; + } + } + return false; +} + auto isOptionalMemberCallWithNameMatcher( ast_matchers::internal::Matcher<NamedDecl> matcher, const std::optional<StatementMatcher> &Ignorable = std::nullopt) { @@ -982,8 +996,9 @@ ignorableOptional(const UncheckedOptionalAccessModelOptions &Options) { StatementMatcher valueCall(const std::optional<StatementMatcher> &IgnorableOptional) { - return isOptionalMemberCallWithNameMatcher(hasName("value"), - IgnorableOptional); + return isOptionalMemberCallWithNameMatcher( + anyOf(hasName("value"), hasAnalyseAsMethodName("value")), + IgnorableOptional); } StatementMatcher diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 243c66b474384..f999c307b7111 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6485,6 +6485,30 @@ static void handleAnalyseAsClass(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(::new (S.Context) AnalyseAsClassAttr(S.Context, AL, Str)); } +// for now this only handles std::optional (POC) +static bool isValidAnalyseAsMethodAttr(Decl *D, StringRef Tag) { + // no validation is done currently. if someone writes something with a nonsense name, + // it simply won't be validated but also no warning will be emitted + // would be nice to do something smarter in the real implementation + return true; +} + +static void handleAnalyseAsMethod(Sema &S, Decl *D, const ParsedAttr &AL) { + StringRef Str; + if (!S.checkStringLiteralArgumentAttr(AL, 0, Str)) + return; + if (D->hasAttr<AnalyseAsMethodAttr>()) { + S.Diag(AL.getLoc(), diag::err_duplicate_attribute) << AL; + return; + } + if (!isValidAnalyseAsMethodAttr(D, Str)) { + S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL; + return; + } + + D->addAttr(::new (S.Context) AnalyseAsMethodAttr(S.Context, AL, Str)); +} + static bool hasBTFDeclTagAttr(Decl *D, StringRef Tag) { for (const auto *I : D->specific_attrs<BTFDeclTagAttr>()) { if (I->getBTFDeclTag() == Tag) @@ -7577,6 +7601,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_AnalyseAsClass: handleAnalyseAsClass(S, D, AL); break; + case ParsedAttr::AT_AnalyseAsMethod: + handleAnalyseAsMethod(S, D, AL); + break; case ParsedAttr::AT_BTFDeclTag: handleBTFDeclTagAttr(S, D, AL); break; diff --git a/hicketts_test/hicketts_optional.h b/hicketts_test/hicketts_optional.h index b915b01f70290..43ec98831a74b 100644 --- a/hicketts_test/hicketts_optional.h +++ b/hicketts_test/hicketts_optional.h @@ -26,11 +26,10 @@ class [[clang::analyse_as_class("std::optional")]] HickettsOptional { HickettsOptional(HickettsOptional &&) = default; // Equivalent to std::optional::value() - //[[clang_analyse_as_method(std::optional::value)]] - const T &unwrap() const & { return *storage_; } - T &unwrap() & { return *storage_; } - const T &&unwrap() const && { return static_cast<const T &&>(*storage_); } - T &&unwrap() && { return static_cast<T &&>(*storage_); } + [[clang::analyse_as_method("std::optional::value")]] const T &unwrap() const & { return *storage_; } + [[clang::analyse_as_method("std::optional::value")]] T &unwrap() & { return *storage_; } + [[clang::analyse_as_method("std::optional::value")]] const T &&unwrap() const && { return static_cast<const T &&>(*storage_); } + [[clang::analyse_as_method("std::optional::value")]] T &&unwrap() && { return static_cast<T &&>(*storage_); } const T &value() const & { return *storage_; } T &value() & { return *storage_; } @@ -38,8 +37,8 @@ class [[clang::analyse_as_class("std::optional")]] HickettsOptional { T &&value() && { return static_cast<T &&>(*storage_); } // Equivalent to std::optional::operator*() - const T &deref() const & { return *storage_; } - T &deref() & { return *storage_; } + [[clang::analyse_as_method("std::optional::operator*")]] const T &deref() const & { return *storage_; } + [[clang::analyse_as_method("std::optional::operator*")]] T &deref() & { return *storage_; } // Equivalent to std::optional::operator->() const T* operator ->() const { return storage_; } @@ -47,10 +46,10 @@ class [[clang::analyse_as_class("std::optional")]] HickettsOptional { const T *arrow() const { return storage_; } T *arrow() { return storage_; } - // Equivalent to std::optional::operator bool / has_value() + // Equivalent to std::optional::operator bool / hasValue() constexpr bool hasValue() const noexcept { return storage_ != nullptr; } constexpr explicit operator bool() const noexcept { return storage_ != nullptr; } - constexpr bool isPresent() const noexcept { return storage_ != nullptr; } + [[clang::analyse_as_method("std::optional::hasValue")]] constexpr bool isPresent() const noexcept { return storage_ != nullptr; } constexpr bool isEmpty() const noexcept { return storage_ == nullptr; } // Equivalent to std::optional::value_or() >From 9d6520a6cac0f322cd67ac5a1619a6b8fd0128cd Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Tue, 28 Apr 2026 17:07:21 +0100 Subject: [PATCH 04/12] extend method matching . --- .../Models/UncheckedOptionalAccessModel.cpp | 11 ++++++----- hicketts_test/hicketts_optional.h | 15 +++++++-------- hicketts_test/test_hicketts_optional.cpp | 10 +++++----- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index f2c9aa756943e..137b44560c529 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -358,7 +358,7 @@ auto isValueOrStringEmptyCall() { callee(cxxMethodDecl(hasName("empty"))), onImplicitObjectArgument(ignoringImplicit( cxxMemberCallExpr(on(expr(unless(cxxThisExpr()))), - callee(cxxMethodDecl(hasName("value_or"), + callee(cxxMethodDecl(anyOf(hasName("value_or"), hasAnalyseAsMethodName("value_or")), ofClass(optionalClass()))), hasArgument(0, stringLiteral(hasSize(0)))) .bind(ValueOrCallID)))); @@ -1071,7 +1071,8 @@ auto buildTransferMatchSwitch() { // will also pass for other types .CaseOfCFGStmt<CXXMemberCallExpr>( isOptionalMemberCallWithNameMatcher( - hasAnyName("has_value", "hasValue")), + anyOf(hasAnyName("has_value", "hasValue"), + hasAnalyseAsMethodName("has_value"))), transferOptionalHasValueCall) // optional::operator bool @@ -1101,7 +1102,7 @@ auto buildTransferMatchSwitch() { // optional::emplace .CaseOfCFGStmt<CXXMemberCallExpr>( - isOptionalMemberCallWithNameMatcher(hasName("emplace")), + isOptionalMemberCallWithNameMatcher(anyOf(hasName("emplace"), hasAnalyseAsMethodName("emplace"))), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { if (RecordStorageLocation *Loc = @@ -1112,7 +1113,7 @@ auto buildTransferMatchSwitch() { // optional::reset .CaseOfCFGStmt<CXXMemberCallExpr>( - isOptionalMemberCallWithNameMatcher(hasName("reset")), + isOptionalMemberCallWithNameMatcher(anyOf(hasName("reset"), hasAnalyseAsMethodName("reset"))), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { if (RecordStorageLocation *Loc = @@ -1124,7 +1125,7 @@ auto buildTransferMatchSwitch() { // optional::swap .CaseOfCFGStmt<CXXMemberCallExpr>( - isOptionalMemberCallWithNameMatcher(hasName("swap")), + isOptionalMemberCallWithNameMatcher(anyOf(hasName("swap"), hasAnalyseAsMethodName("swap"))), transferSwapCall) // std::swap diff --git a/hicketts_test/hicketts_optional.h b/hicketts_test/hicketts_optional.h index 43ec98831a74b..2c6978a7e2339 100644 --- a/hicketts_test/hicketts_optional.h +++ b/hicketts_test/hicketts_optional.h @@ -37,8 +37,8 @@ class [[clang::analyse_as_class("std::optional")]] HickettsOptional { T &&value() && { return static_cast<T &&>(*storage_); } // Equivalent to std::optional::operator*() - [[clang::analyse_as_method("std::optional::operator*")]] const T &deref() const & { return *storage_; } - [[clang::analyse_as_method("std::optional::operator*")]] T &deref() & { return *storage_; } + [[clang::analyse_as_method("std::optional::value")]] const T &deref() const & { return *storage_; } + [[clang::analyse_as_method("std::optional::value")]] T &deref() & { return *storage_; } // Equivalent to std::optional::operator->() const T* operator ->() const { return storage_; } @@ -47,10 +47,9 @@ class [[clang::analyse_as_class("std::optional")]] HickettsOptional { T *arrow() { return storage_; } // Equivalent to std::optional::operator bool / hasValue() - constexpr bool hasValue() const noexcept { return storage_ != nullptr; } + constexpr bool has_value() const noexcept { return storage_ != nullptr; } constexpr explicit operator bool() const noexcept { return storage_ != nullptr; } - [[clang::analyse_as_method("std::optional::hasValue")]] constexpr bool isPresent() const noexcept { return storage_ != nullptr; } - constexpr bool isEmpty() const noexcept { return storage_ == nullptr; } + [[clang::analyse_as_method("std::optional::has_value")]] constexpr bool isPresent() const noexcept { return storage_ != nullptr; } // Equivalent to std::optional::value_or() template <typename U> @@ -60,13 +59,13 @@ class [[clang::analyse_as_class("std::optional")]] HickettsOptional { // Equivalent to std::optional::emplace() template <typename... Args> - T &construct(Args &&...args) { return *storage_; } + [[clang::analyse_as_method("std::optional::emplace")]] T &construct(Args &&...args) { return *storage_; } // Equivalent to std::optional::reset() - void clear() noexcept { storage_ = nullptr; } + [[clang::analyse_as_method("std::optional::reset")]] void clear() noexcept { storage_ = nullptr; } // Equivalent to std::optional::swap() - void exchange(HickettsOptional &other) noexcept { + [[clang::analyse_as_method("std::optional::swap")]] void exchange(HickettsOptional &other) noexcept { T *tmp = storage_; storage_ = other.storage_; other.storage_ = tmp; diff --git a/hicketts_test/test_hicketts_optional.cpp b/hicketts_test/test_hicketts_optional.cpp index fa0153a79f341..1ff16d13fe174 100644 --- a/hicketts_test/test_hicketts_optional.cpp +++ b/hicketts_test/test_hicketts_optional.cpp @@ -30,7 +30,7 @@ static void checkedWithBool(mylib::HickettsOptional<int> &Val) { } static void checkedValueWithBool(mylib::HickettsOptional<int> &Val) { - if (Val.hasValue()) { + if (Val.has_value()) { Val.value(); // safe — checked via operator bool } } @@ -41,11 +41,11 @@ static void checkedWithIsPresent(mylib::HickettsOptional<int> &Val) { } } -static void checkedWithIsEmpty(mylib::HickettsOptional<int> &Val) { +/* static void checkedWithIsEmpty(mylib::HickettsOptional<int> &Val) { if (!Val.isEmpty()) { Val.unwrap(); // safe — checked via !isEmpty() } -} +} NYI */ // --- State changes --- @@ -70,12 +70,12 @@ static void unsafeAfterExchange(mylib::HickettsOptional<int> &A, // --- Guarded paths --- -static void constructCoversEmptyBranch(mylib::HickettsOptional<int> &Val) { +/*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 >From 34ce7b2b289e5e5a4cb0d708d34f4675d7bdc426 Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Thu, 30 Apr 2026 12:48:57 +0100 Subject: [PATCH 05/12] missed a bit --- clang/include/clang/Basic/Attr.td | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 97536ac7a1966..adc127df5756e 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -921,6 +921,22 @@ def AlignValue : Attr { let Documentation = [AlignValueDocs]; } +def AnalyseAsClass : InheritableAttr { + let Spellings = [Clang<"analyse_as_class">]; + let Args = [StringArgument<"ClassName">]; + let Subjects = SubjectList<[Record], + ErrorDiag>; + let Documentation = [Undocumented]; +} + +def AnalyseAsMethod : InheritableAttr { + let Spellings = [Clang<"analyse_as_method">]; + let Args = [StringArgument<"MethodName">]; + let Subjects = SubjectList<[CXXMethod], + ErrorDiag>; + let Documentation = [Undocumented]; +} + def AlignMac68k : InheritableAttr { // This attribute has no spellings as it is only ever created implicitly. let Spellings = []; >From 3532f8f3ae825e185f7f5c3506e3b96fbd16dda3 Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Fri, 1 May 2026 15:40:38 +0100 Subject: [PATCH 06/12] add unit tests --- .../UncheckedOptionalAccessModelTest.cpp | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index 074dc95e2c388..72cf12bcd0817 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -946,7 +946,7 @@ TEST_P(UncheckedOptionalAccessTest, OptionalReturnedFromFuntionCall) { ExpectDiagnosticsFor( R"( #include "unchecked_optional_access_test.h" - + struct S { $ns::$optional<float> x; } s; @@ -2860,14 +2860,14 @@ TEST_P( struct B { const A& getA() const { return a; } - void callWithoutChanges() const { - // no-op + void callWithoutChanges() const { + // no-op } A a; }; - void target(B& b) { + void target(B& b) { if (b.getA().get().has_value()) { b.callWithoutChanges(); // calling const method which cannot change A b.getA().get().value(); @@ -2995,6 +2995,48 @@ TEST_P(UncheckedOptionalAccessTest, AssertFalseGtestMacroWithNullableValue) { )cc"); } +TEST_P(UncheckedOptionalAccessTest, TestCustomAttributeBalik) { + ExpectDiagnosticsFor(R"cc( + #include "unchecked_optional_access_test.h" + + template <typename T> + class __attribute__((analyse_as_class("std::optional"))) MyOptional { + public: + bool has_value() const; + T& value(); + const T& value() const; + }; + + void target(MyOptional<int> opt) { + opt.value(); // [[unsafe]] + if (opt.has_value()) { + opt.value(); + } + } + )cc"); +} + +TEST_P(UncheckedOptionalAccessTest, TestCustomAttributeHicketts) { + ExpectDiagnosticsFor(R"cc( + #include "unchecked_optional_access_test.h" + + template <typename T> + class __attribute__((analyse_as_class("std::optional"))) MyOptional { + public: + __attribute__((analyse_as_method("std::optional::has_value"))) bool isNotNull() const; + __attribute__((analyse_as_method("std::optional::value"))) T& unwrap(); + __attribute__((analyse_as_method("std::optional::value"))) const T& unwrap() const; + }; + + void target(MyOptional<int> opt) { + opt.unwrap(); // [[unsafe]] + if (opt.isNotNull()) { + opt.unwrap(); + } + } + )cc"); +} + // FIXME: Add support for: // - constructors (copy, move) // - assignment operators (default, copy, move) >From 7320c8c063be143928434c9046eb85cf7e22ed78 Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Fri, 1 May 2026 16:01:22 +0100 Subject: [PATCH 07/12] . --- .../bugprone/unchecked-optional-access.cpp | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp index 337474bdf7535..cdd8fdb82174b 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp @@ -384,3 +384,72 @@ void foo() { if (!vec.empty()) vec[0].x = 0; } + +// Custom optional-like type using analyse_as_class / analyse_as_method attributes. +template <typename T> +class [[clang::analyse_as_class("std::optional")]] CustomOptional { +public: + [[clang::analyse_as_method("std::optional::has_value")]] bool isEngaged() const; + [[clang::analyse_as_method("std::optional::value")]] T &retrieve(); + [[clang::analyse_as_method("std::optional::value")]] const T &retrieve() const; + [[clang::analyse_as_method("std::optional::emplace")]] T &fill(T val); + [[clang::analyse_as_method("std::optional::reset")]] void clear(); +}; + +void custom_unchecked_access(CustomOptional<int> opt) { + opt.retrieve(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] +} + +void custom_checked_access(CustomOptional<int> opt) { + if (opt.isEngaged()) { + opt.retrieve(); + } +} + +void custom_emplace_then_access(CustomOptional<int> opt) { + opt.fill(42); + opt.retrieve(); +} + +void custom_clear_then_access(CustomOptional<int> opt) { + opt.fill(42); + opt.clear(); + opt.retrieve(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] +} + +// Custom optional-like type using standard method names — recognised via +// analyse_as_class alone, without analyse_as_method attributes. +template <typename T> +class [[clang::analyse_as_class("std::optional")]] StdNamedOptional { +public: + bool has_value() const; + T &value(); + const T &value() const; + T &emplace(T val); + void reset(); +}; + +void std_named_unchecked_access(StdNamedOptional<int> opt) { + opt.value(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] +} + +void std_named_checked_access(StdNamedOptional<int> opt) { + if (opt.has_value()) { + opt.value(); + } +} + +void std_named_emplace_then_access(StdNamedOptional<int> opt) { + opt.emplace(42); + opt.value(); +} + +void std_named_reset_then_access(StdNamedOptional<int> opt) { + opt.emplace(42); + opt.reset(); + opt.value(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] +} >From dc6922262ffdaba0b0202b93e82f320286249bca Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Fri, 1 May 2026 16:16:29 +0100 Subject: [PATCH 08/12] . --- hicketts_test/hicketts_optional.h | 81 ----------------------- hicketts_test/test_hicketts_optional.cpp | 83 ------------------------ 2 files changed, 164 deletions(-) delete mode 100644 hicketts_test/hicketts_optional.h delete mode 100644 hicketts_test/test_hicketts_optional.cpp diff --git a/hicketts_test/hicketts_optional.h b/hicketts_test/hicketts_optional.h deleted file mode 100644 index 2c6978a7e2339..0000000000000 --- a/hicketts_test/hicketts_optional.h +++ /dev/null @@ -1,81 +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::analyse_as_class("std::optional")]] HickettsOptional { - T *storage_ = nullptr; - -public: - constexpr HickettsOptional() noexcept {} - - constexpr HickettsOptional(nothing_t) noexcept {} - - HickettsOptional(const HickettsOptional &) = default; - - HickettsOptional(HickettsOptional &&) = default; - - // Equivalent to std::optional::value() - [[clang::analyse_as_method("std::optional::value")]] const T &unwrap() const & { return *storage_; } - [[clang::analyse_as_method("std::optional::value")]] T &unwrap() & { return *storage_; } - [[clang::analyse_as_method("std::optional::value")]] const T &&unwrap() const && { return static_cast<const T &&>(*storage_); } - [[clang::analyse_as_method("std::optional::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::analyse_as_method("std::optional::value")]] const T &deref() const & { return *storage_; } - [[clang::analyse_as_method("std::optional::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::analyse_as_method("std::optional::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::analyse_as_method("std::optional::emplace")]] T &construct(Args &&...args) { return *storage_; } - - // Equivalent to std::optional::reset() - [[clang::analyse_as_method("std::optional::reset")]] void clear() noexcept { storage_ = nullptr; } - - // Equivalent to std::optional::swap() - [[clang::analyse_as_method("std::optional::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; } -}; - -} // namespace mylib - -#endif // HICKETTS_OPTIONAL_H_ diff --git a/hicketts_test/test_hicketts_optional.cpp b/hicketts_test/test_hicketts_optional.cpp deleted file mode 100644 index 1ff16d13fe174..0000000000000 --- a/hicketts_test/test_hicketts_optional.cpp +++ /dev/null @@ -1,83 +0,0 @@ -// Test cases for mylib::HickettsOptional — a custom optional-like type -// with differently named functions. -// -// Run from hicketts_test/ with: -// clang-tidy -checks='bugprone-unchecked-optional-access' \ -// test_hicketts_optional.cpp -- -I . -Wno-undefined-inline - -#include "hicketts_optional.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 --- - -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 - } -} - -// --- 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; -} >From e50be59fa64622856902e3b0a80d1e8c1699d9ec Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Fri, 1 May 2026 17:28:47 +0100 Subject: [PATCH 09/12] comment possible cody tidy up locations --- .../Models/UncheckedOptionalAccessModel.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 137b44560c529..1fee746ff7a97 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -71,6 +71,8 @@ static bool hasOptionalClassName(const CXXRecordDecl &RD) { return false; } + // this code could be removed if base::Optional and folly::Optional used + // [[clang::analyse_as_class("std::optional")]] if (RD.getName() == "Optional") { // Check whether namespace is "::base" or "::folly". const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext()); @@ -78,12 +80,16 @@ static bool hasOptionalClassName(const CXXRecordDecl &RD) { isFullyQualifiedNamespaceEqualTo(*N, "folly")); } + // this code could be removed if Optional_Base used + // [[clang::analyse_as_class("std::optional")]] if (RD.getName() == "Optional_Base") { const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext()); return N != nullptr && isFullyQualifiedNamespaceEqualTo(*N, "bslstl", "BloombergLP"); } + // this code could be removed if NullableValue used + // [[clang::analyse_as_class("std::optional")]] if (RD.getName() == "NullableValue") { const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext()); return N != nullptr && @@ -1069,6 +1075,8 @@ auto buildTransferMatchSwitch() { // optional::has_value, optional::hasValue // Of the supported optionals only folly::Optional uses hasValue, but this // will also pass for other types + // "hasValue" could be removed if folly::Optional used + // [[clang::analyse_as_method("std::optional::has_value")]] on hasValue() .CaseOfCFGStmt<CXXMemberCallExpr>( isOptionalMemberCallWithNameMatcher( anyOf(hasAnyName("has_value", "hasValue"), @@ -1080,12 +1088,16 @@ auto buildTransferMatchSwitch() { isOptionalMemberCallWithNameMatcher(hasName("operator bool")), transferOptionalHasValueCall) + // this code could be removed if NullableValue used + // [[clang::analyse_as_inverse_method("std::optional::has_value")]] on isNull() *NYI // NullableValue::isNull // Only NullableValue has isNull .CaseOfCFGStmt<CXXMemberCallExpr>( isOptionalMemberCallWithNameMatcher(hasName("isNull")), transferOptionalIsNullCall) + // this code could be removed if NullableValue used + // [[clang::analyse_as_method("std::optional::emplace")]] on makeValue() and makeValueInplace() // NullableValue::makeValue, NullableValue::makeValueInplace // Only NullableValue has these methods, but this // will also pass for other types >From a0d0dd9c93a309410c15030774368af98c425e1e Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Fri, 1 May 2026 17:51:13 +0100 Subject: [PATCH 10/12] remove fully qualified names for method attributes --- .../checkers/bugprone/unchecked-optional-access.cpp | 10 +++++----- .../Models/UncheckedOptionalAccessModel.cpp | 10 +++------- .../FlowSensitive/UncheckedOptionalAccessModelTest.cpp | 6 +++--- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp index cdd8fdb82174b..bea941ff2104d 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp @@ -389,11 +389,11 @@ void foo() { template <typename T> class [[clang::analyse_as_class("std::optional")]] CustomOptional { public: - [[clang::analyse_as_method("std::optional::has_value")]] bool isEngaged() const; - [[clang::analyse_as_method("std::optional::value")]] T &retrieve(); - [[clang::analyse_as_method("std::optional::value")]] const T &retrieve() const; - [[clang::analyse_as_method("std::optional::emplace")]] T &fill(T val); - [[clang::analyse_as_method("std::optional::reset")]] void clear(); + [[clang::analyse_as_method("has_value")]] bool isEngaged() const; + [[clang::analyse_as_method("value")]] T &retrieve(); + [[clang::analyse_as_method("value")]] const T &retrieve() const; + [[clang::analyse_as_method("emplace")]] T &fill(T val); + [[clang::analyse_as_method("reset")]] void clear(); }; void custom_unchecked_access(CustomOptional<int> opt) { diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 1fee746ff7a97..2264c7e60f190 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -240,11 +240,7 @@ AST_MATCHER_P(NamedDecl, hasAnalyseAsMethodName, std::string, MethodName) { if (const auto *MD = dyn_cast<CXXMethodDecl>(&Node)) { if (const auto *Attr = MD->getAttr<AnalyseAsMethodAttr>()) { StringRef AttrValue = Attr->getMethodName(); - auto Pos = AttrValue.rfind("::"); - StringRef AttrMethodName = (Pos != StringRef::npos) - ? AttrValue.substr(Pos + 2) - : AttrValue; - return AttrMethodName == MethodName; + return AttrValue == MethodName; } } return false; @@ -1076,7 +1072,7 @@ auto buildTransferMatchSwitch() { // Of the supported optionals only folly::Optional uses hasValue, but this // will also pass for other types // "hasValue" could be removed if folly::Optional used - // [[clang::analyse_as_method("std::optional::has_value")]] on hasValue() + // [[clang::analyse_as_method("has_value")]] on hasValue() .CaseOfCFGStmt<CXXMemberCallExpr>( isOptionalMemberCallWithNameMatcher( anyOf(hasAnyName("has_value", "hasValue"), @@ -1097,7 +1093,7 @@ auto buildTransferMatchSwitch() { transferOptionalIsNullCall) // this code could be removed if NullableValue used - // [[clang::analyse_as_method("std::optional::emplace")]] on makeValue() and makeValueInplace() + // [[clang::analyse_as_method("emplace")]] on makeValue() and makeValueInplace() // NullableValue::makeValue, NullableValue::makeValueInplace // Only NullableValue has these methods, but this // will also pass for other types diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index 72cf12bcd0817..ac67bda300b65 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -3023,9 +3023,9 @@ TEST_P(UncheckedOptionalAccessTest, TestCustomAttributeHicketts) { template <typename T> class __attribute__((analyse_as_class("std::optional"))) MyOptional { public: - __attribute__((analyse_as_method("std::optional::has_value"))) bool isNotNull() const; - __attribute__((analyse_as_method("std::optional::value"))) T& unwrap(); - __attribute__((analyse_as_method("std::optional::value"))) const T& unwrap() const; + __attribute__((analyse_as_method("has_value"))) bool isNotNull() const; + __attribute__((analyse_as_method("value"))) T& unwrap(); + __attribute__((analyse_as_method("value"))) const T& unwrap() const; }; void target(MyOptional<int> opt) { >From 7263f13aea755ba43cbf6efcfafeaa2326cc2d59 Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Fri, 1 May 2026 17:55:12 +0100 Subject: [PATCH 11/12] American spelling is just cheating at Scrabble... --- .../bugprone/unchecked-optional-access.cpp | 18 +++++------ clang/include/clang/Basic/Attr.td | 8 ++--- .../Models/UncheckedOptionalAccessModel.cpp | 30 +++++++++---------- clang/lib/Sema/SemaDeclAttr.cpp | 28 ++++++++--------- .../UncheckedOptionalAccessModelTest.cpp | 10 +++---- 5 files changed, 47 insertions(+), 47 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp index bea941ff2104d..6d68b95697925 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp @@ -385,15 +385,15 @@ void foo() { vec[0].x = 0; } -// Custom optional-like type using analyse_as_class / analyse_as_method attributes. +// Custom optional-like type using analyze_as_class / analyze_as_method attributes. template <typename T> -class [[clang::analyse_as_class("std::optional")]] CustomOptional { +class [[clang::analyze_as_class("std::optional")]] CustomOptional { public: - [[clang::analyse_as_method("has_value")]] bool isEngaged() const; - [[clang::analyse_as_method("value")]] T &retrieve(); - [[clang::analyse_as_method("value")]] const T &retrieve() const; - [[clang::analyse_as_method("emplace")]] T &fill(T val); - [[clang::analyse_as_method("reset")]] void clear(); + [[clang::analyze_as_method("has_value")]] bool isEngaged() const; + [[clang::analyze_as_method("value")]] T &retrieve(); + [[clang::analyze_as_method("value")]] const T &retrieve() const; + [[clang::analyze_as_method("emplace")]] T &fill(T val); + [[clang::analyze_as_method("reset")]] void clear(); }; void custom_unchecked_access(CustomOptional<int> opt) { @@ -420,9 +420,9 @@ void custom_clear_then_access(CustomOptional<int> opt) { } // Custom optional-like type using standard method names — recognised via -// analyse_as_class alone, without analyse_as_method attributes. +// analyze_as_class alone, without analyze_as_method attributes. template <typename T> -class [[clang::analyse_as_class("std::optional")]] StdNamedOptional { +class [[clang::analyze_as_class("std::optional")]] StdNamedOptional { public: bool has_value() const; T &value(); diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index adc127df5756e..7db521598bff7 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -921,16 +921,16 @@ def AlignValue : Attr { let Documentation = [AlignValueDocs]; } -def AnalyseAsClass : InheritableAttr { - let Spellings = [Clang<"analyse_as_class">]; +def AnalyzeAsClass : InheritableAttr { + let Spellings = [Clang<"analyze_as_class">]; let Args = [StringArgument<"ClassName">]; let Subjects = SubjectList<[Record], ErrorDiag>; let Documentation = [Undocumented]; } -def AnalyseAsMethod : InheritableAttr { - let Spellings = [Clang<"analyse_as_method">]; +def AnalyzeAsMethod : InheritableAttr { + let Spellings = [Clang<"analyze_as_method">]; let Args = [StringArgument<"MethodName">]; let Subjects = SubjectList<[CXXMethod], ErrorDiag>; diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 2264c7e60f190..49743b94eebeb 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -72,7 +72,7 @@ static bool hasOptionalClassName(const CXXRecordDecl &RD) { } // this code could be removed if base::Optional and folly::Optional used - // [[clang::analyse_as_class("std::optional")]] + // [[clang::analyze_as_class("std::optional")]] if (RD.getName() == "Optional") { // Check whether namespace is "::base" or "::folly". const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext()); @@ -81,7 +81,7 @@ static bool hasOptionalClassName(const CXXRecordDecl &RD) { } // this code could be removed if Optional_Base used - // [[clang::analyse_as_class("std::optional")]] + // [[clang::analyze_as_class("std::optional")]] if (RD.getName() == "Optional_Base") { const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext()); return N != nullptr && @@ -89,14 +89,14 @@ static bool hasOptionalClassName(const CXXRecordDecl &RD) { } // this code could be removed if NullableValue used - // [[clang::analyse_as_class("std::optional")]] + // [[clang::analyze_as_class("std::optional")]] if (RD.getName() == "NullableValue") { const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext()); return N != nullptr && isFullyQualifiedNamespaceEqualTo(*N, "bdlb", "BloombergLP"); } - if (RD.hasAttr<AnalyseAsClassAttr>()) + if (RD.hasAttr<AnalyzeAsClassAttr>()) return true; return false; @@ -236,9 +236,9 @@ AST_MATCHER(CXXOperatorCallExpr, hasOptionalOperatorObjectType) { return hasReceiverTypeDesugaringToOptional(Node.getArg(0)); } -AST_MATCHER_P(NamedDecl, hasAnalyseAsMethodName, std::string, MethodName) { +AST_MATCHER_P(NamedDecl, hasAnalyzeAsMethodName, std::string, MethodName) { if (const auto *MD = dyn_cast<CXXMethodDecl>(&Node)) { - if (const auto *Attr = MD->getAttr<AnalyseAsMethodAttr>()) { + if (const auto *Attr = MD->getAttr<AnalyzeAsMethodAttr>()) { StringRef AttrValue = Attr->getMethodName(); return AttrValue == MethodName; } @@ -360,7 +360,7 @@ auto isValueOrStringEmptyCall() { callee(cxxMethodDecl(hasName("empty"))), onImplicitObjectArgument(ignoringImplicit( cxxMemberCallExpr(on(expr(unless(cxxThisExpr()))), - callee(cxxMethodDecl(anyOf(hasName("value_or"), hasAnalyseAsMethodName("value_or")), + callee(cxxMethodDecl(anyOf(hasName("value_or"), hasAnalyzeAsMethodName("value_or")), ofClass(optionalClass()))), hasArgument(0, stringLiteral(hasSize(0)))) .bind(ValueOrCallID)))); @@ -999,7 +999,7 @@ ignorableOptional(const UncheckedOptionalAccessModelOptions &Options) { StatementMatcher valueCall(const std::optional<StatementMatcher> &IgnorableOptional) { return isOptionalMemberCallWithNameMatcher( - anyOf(hasName("value"), hasAnalyseAsMethodName("value")), + anyOf(hasName("value"), hasAnalyzeAsMethodName("value")), IgnorableOptional); } @@ -1072,11 +1072,11 @@ auto buildTransferMatchSwitch() { // Of the supported optionals only folly::Optional uses hasValue, but this // will also pass for other types // "hasValue" could be removed if folly::Optional used - // [[clang::analyse_as_method("has_value")]] on hasValue() + // [[clang::analyze_as_method("has_value")]] on hasValue() .CaseOfCFGStmt<CXXMemberCallExpr>( isOptionalMemberCallWithNameMatcher( anyOf(hasAnyName("has_value", "hasValue"), - hasAnalyseAsMethodName("has_value"))), + hasAnalyzeAsMethodName("has_value"))), transferOptionalHasValueCall) // optional::operator bool @@ -1085,7 +1085,7 @@ auto buildTransferMatchSwitch() { transferOptionalHasValueCall) // this code could be removed if NullableValue used - // [[clang::analyse_as_inverse_method("std::optional::has_value")]] on isNull() *NYI + // [[clang::analyze_as_inverse_method("std::optional::has_value")]] on isNull() *NYI // NullableValue::isNull // Only NullableValue has isNull .CaseOfCFGStmt<CXXMemberCallExpr>( @@ -1093,7 +1093,7 @@ auto buildTransferMatchSwitch() { transferOptionalIsNullCall) // this code could be removed if NullableValue used - // [[clang::analyse_as_method("emplace")]] on makeValue() and makeValueInplace() + // [[clang::analyze_as_method("emplace")]] on makeValue() and makeValueInplace() // NullableValue::makeValue, NullableValue::makeValueInplace // Only NullableValue has these methods, but this // will also pass for other types @@ -1110,7 +1110,7 @@ auto buildTransferMatchSwitch() { // optional::emplace .CaseOfCFGStmt<CXXMemberCallExpr>( - isOptionalMemberCallWithNameMatcher(anyOf(hasName("emplace"), hasAnalyseAsMethodName("emplace"))), + isOptionalMemberCallWithNameMatcher(anyOf(hasName("emplace"), hasAnalyzeAsMethodName("emplace"))), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { if (RecordStorageLocation *Loc = @@ -1121,7 +1121,7 @@ auto buildTransferMatchSwitch() { // optional::reset .CaseOfCFGStmt<CXXMemberCallExpr>( - isOptionalMemberCallWithNameMatcher(anyOf(hasName("reset"), hasAnalyseAsMethodName("reset"))), + isOptionalMemberCallWithNameMatcher(anyOf(hasName("reset"), hasAnalyzeAsMethodName("reset"))), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { if (RecordStorageLocation *Loc = @@ -1133,7 +1133,7 @@ auto buildTransferMatchSwitch() { // optional::swap .CaseOfCFGStmt<CXXMemberCallExpr>( - isOptionalMemberCallWithNameMatcher(anyOf(hasName("swap"), hasAnalyseAsMethodName("swap"))), + isOptionalMemberCallWithNameMatcher(anyOf(hasName("swap"), hasAnalyzeAsMethodName("swap"))), transferSwapCall) // std::swap diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index f999c307b7111..0aba360f7b7c2 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6463,50 +6463,50 @@ static void handleAbiTagAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } // for now this only handles std::optional (POC) -static bool isValidAnalyseAsClassAttr(Decl *D, StringRef Tag) { +static bool isValidAnalyzeAsClassAttr(Decl *D, StringRef Tag) { if (Tag == "std::optional") return true; return false; } -static void handleAnalyseAsClass(Sema &S, Decl *D, const ParsedAttr &AL) { +static void handleAnalyzeAsClass(Sema &S, Decl *D, const ParsedAttr &AL) { StringRef Str; if (!S.checkStringLiteralArgumentAttr(AL, 0, Str)) return; - if (D->hasAttr<AnalyseAsClassAttr>()) { + if (D->hasAttr<AnalyzeAsClassAttr>()) { S.Diag(AL.getLoc(), diag::err_duplicate_attribute) << AL; return; } - if (!isValidAnalyseAsClassAttr(D, Str)) { + if (!isValidAnalyzeAsClassAttr(D, Str)) { S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL; return; } - D->addAttr(::new (S.Context) AnalyseAsClassAttr(S.Context, AL, Str)); + D->addAttr(::new (S.Context) AnalyzeAsClassAttr(S.Context, AL, Str)); } // for now this only handles std::optional (POC) -static bool isValidAnalyseAsMethodAttr(Decl *D, StringRef Tag) { +static bool isValidAnalyzeAsMethodAttr(Decl *D, StringRef Tag) { // no validation is done currently. if someone writes something with a nonsense name, // it simply won't be validated but also no warning will be emitted // would be nice to do something smarter in the real implementation return true; } -static void handleAnalyseAsMethod(Sema &S, Decl *D, const ParsedAttr &AL) { +static void handleAnalyzeAsMethod(Sema &S, Decl *D, const ParsedAttr &AL) { StringRef Str; if (!S.checkStringLiteralArgumentAttr(AL, 0, Str)) return; - if (D->hasAttr<AnalyseAsMethodAttr>()) { + if (D->hasAttr<AnalyzeAsMethodAttr>()) { S.Diag(AL.getLoc(), diag::err_duplicate_attribute) << AL; return; } - if (!isValidAnalyseAsMethodAttr(D, Str)) { + if (!isValidAnalyzeAsMethodAttr(D, Str)) { S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL; return; } - D->addAttr(::new (S.Context) AnalyseAsMethodAttr(S.Context, AL, Str)); + D->addAttr(::new (S.Context) AnalyzeAsMethodAttr(S.Context, AL, Str)); } static bool hasBTFDeclTagAttr(Decl *D, StringRef Tag) { @@ -7598,11 +7598,11 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_BPFPreserveStaticOffset: handleSimpleAttribute<BPFPreserveStaticOffsetAttr>(S, D, AL); break; - case ParsedAttr::AT_AnalyseAsClass: - handleAnalyseAsClass(S, D, AL); + case ParsedAttr::AT_AnalyzeAsClass: + handleAnalyzeAsClass(S, D, AL); break; - case ParsedAttr::AT_AnalyseAsMethod: - handleAnalyseAsMethod(S, D, AL); + case ParsedAttr::AT_AnalyzeAsMethod: + handleAnalyzeAsMethod(S, D, AL); break; case ParsedAttr::AT_BTFDeclTag: handleBTFDeclTagAttr(S, D, AL); diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index ac67bda300b65..5d224ceb7cf3f 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -3000,7 +3000,7 @@ TEST_P(UncheckedOptionalAccessTest, TestCustomAttributeBalik) { #include "unchecked_optional_access_test.h" template <typename T> - class __attribute__((analyse_as_class("std::optional"))) MyOptional { + class __attribute__((analyze_as_class("std::optional"))) MyOptional { public: bool has_value() const; T& value(); @@ -3021,11 +3021,11 @@ TEST_P(UncheckedOptionalAccessTest, TestCustomAttributeHicketts) { #include "unchecked_optional_access_test.h" template <typename T> - class __attribute__((analyse_as_class("std::optional"))) MyOptional { + class __attribute__((analyze_as_class("std::optional"))) MyOptional { public: - __attribute__((analyse_as_method("has_value"))) bool isNotNull() const; - __attribute__((analyse_as_method("value"))) T& unwrap(); - __attribute__((analyse_as_method("value"))) const T& unwrap() const; + __attribute__((analyze_as_method("has_value"))) bool isNotNull() const; + __attribute__((analyze_as_method("value"))) T& unwrap(); + __attribute__((analyze_as_method("value"))) const T& unwrap() const; }; void target(MyOptional<int> opt) { >From c8d184fa6bc3d4c44cb0456f06e610d2c094277f Mon Sep 17 00:00:00 2001 From: khickett <[email protected]> Date: Fri, 10 Jul 2026 15:03:26 +0100 Subject: [PATCH 12/12] add my test stuff and some test work around constructors --- .../Models/UncheckedOptionalAccessModel.cpp | 11 +- clang/lib/Sema/SemaDeclAttr.cpp | 53 ++++- hicketts/constructors.md | 219 ++++++++++++++++++ hicketts/hicketts_optional.h | 109 +++++++++ hicketts/isValidAnalyzeAsMethodAttr.cpp | 53 +++++ hicketts/plan.md | 187 +++++++++++++++ hicketts/test_hicketts_optional.cpp | 108 +++++++++ warnings.log | 18 ++ 8 files changed, 753 insertions(+), 5 deletions(-) create mode 100644 hicketts/constructors.md create mode 100644 hicketts/hicketts_optional.h create mode 100644 hicketts/isValidAnalyzeAsMethodAttr.cpp create mode 100644 hicketts/plan.md create mode 100644 hicketts/test_hicketts_optional.cpp create mode 100644 warnings.log diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 49743b94eebeb..0f448a60dd6e4 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -236,11 +236,13 @@ AST_MATCHER(CXXOperatorCallExpr, hasOptionalOperatorObjectType) { return hasReceiverTypeDesugaringToOptional(Node.getArg(0)); } -AST_MATCHER_P(NamedDecl, hasAnalyzeAsMethodName, std::string, MethodName) { +AST_MATCHER_P(NamedDecl, hasAnalyzeAsMethodName, std::string, MethodMatcher) { if (const auto *MD = dyn_cast<CXXMethodDecl>(&Node)) { if (const auto *Attr = MD->getAttr<AnalyzeAsMethodAttr>()) { StringRef AttrValue = Attr->getMethodName(); - return AttrValue == MethodName; + if (StringRef(MethodMatcher).contains('(')) + return AttrValue == MethodMatcher; // method + args + return AttrValue.split('(').first == MethodMatcher; // just method name } } return false; @@ -286,8 +288,9 @@ auto inPlaceClass() { auto isOptionalNulloptConstructor() { return cxxConstructExpr( - hasDeclaration(cxxConstructorDecl(parameterCountIs(1), - hasParameter(0, hasNulloptType()))), + hasDeclaration(cxxConstructorDecl( + anyOf(allOf(parameterCountIs(1), hasParameter(0, hasNulloptType())), + hasAnalyzeAsMethodName("optional(std::nullopt_t)")))), hasOptionalOrDerivedType()); } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 0aba360f7b7c2..f79429ecde414 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6490,7 +6490,58 @@ static bool isValidAnalyzeAsMethodAttr(Decl *D, StringRef Tag) { // no validation is done currently. if someone writes something with a nonsense name, // it simply won't be validated but also no warning will be emitted // would be nice to do something smarter in the real implementation - return true; + + if(Tag.empty()) + return false; + + size_t OpenParen = Tag.find('('); + if(OpenParen == StringRef::npos) + return true; // simple function name with no arguments + + if(OpenParen == 0) + return false; // no method name before parentheses + + if (Tag.back() != ')') + return false; // some trailing rubbish after close parenthesis + + StringRef AllParams = Tag.slice(OpenParen + 1, Tag.size() - 1); + + llvm::errs() << "AllParams: " << AllParams << "\n"; + + if (AllParams.empty()) + return true; // verbose but valid + + // look through all the parameters + // check () and <> are balanced + // split on ',' + SmallVector<char, 4> BalanceCheckStack; + size_t SegStart = 0; + + // TODO currently something like blah<,> would pass validation + // we should improve this past POC stage + for (size_t I = 0, End = AllParams.size(); I != End; ++I ) + { + char C = AllParams[I]; + if(C == '('|| C == '<'){ + BalanceCheckStack.push_back(C); + } + else if(C == ')') + { + if(BalanceCheckStack.empty() || BalanceCheckStack.back() != '(') + return false; + BalanceCheckStack.pop_back(); + } + else if(C == '>'){ + if(BalanceCheckStack.empty() || BalanceCheckStack.back() != '<') + return false; + BalanceCheckStack.pop_back(); + } + else if(C == ',' && BalanceCheckStack.empty()){ + // TODO more detailed param checks + } + } + + return BalanceCheckStack.empty(); } static void handleAnalyzeAsMethod(Sema &S, Decl *D, const ParsedAttr &AL) { diff --git a/hicketts/constructors.md b/hicketts/constructors.md new file mode 100644 index 0000000000000..bf667d8341e2a --- /dev/null +++ b/hicketts/constructors.md @@ -0,0 +1,219 @@ +# Handling Overloads and Constructors in `analyze_as_method` + +## Problem + +The current `analyze_as_method("name")` attribute identifies target methods by name alone. This is insufficient when the target class has overloaded methods with different semantics — e.g. `set()` (resets) vs `set(T val)` (assigns). Since `analyze_as_class` is being generalised beyond `std::optional` to support any standard class, overloads are common and unavoidable. + +Constructors are a special case of the same problem: they share a name but have fundamentally different semantics depending on their parameters (e.g. `optional(nullopt_t)` vs `optional(T&&)` vs `optional(in_place_t, Args...)`). + +## Design Constraints + +- The annotated (client) method must not be constrained to have the same parameter types as the target method. The whole point of annotations is that the custom class can have a completely different interface. +- The solution must work for both regular method overloads and constructors. +- The attribute should be the single source of truth for which target overload is intended. + +## Proposed Solution: Signature Strings + +Extend the `analyze_as_method` attribute string to include a parenthesised parameter list when disambiguation is needed: + +```cpp +class [[clang::analyze_as_class("std::vector")]] MyVec { + + // Unambiguous — no signature needed + [[clang::analyze_as_method("clear")]] + void wipe(); + + // Ambiguous — signature required + [[clang::analyze_as_method("insert(iterator, const T&)")]] + void shove_it_in(MyWeirdIter pos, MyT val, bool log = false); + + [[clang::analyze_as_method("insert(iterator, size_type, const T&)")]] + void fill_insert(MyWeirdIter pos, int count, MyT val); +}; +``` + +For constructors, the same mechanism applies — constructors are just methods named with the class name or a reserved keyword: + +```cpp +class [[clang::analyze_as_class("std::optional")]] MyOpt { + + [[clang::analyze_as_method("optional(nullopt_t)")]] + MyOpt(my_null_t); + + [[clang::analyze_as_method("optional(T&&)")]] + MyOpt(MyT&& val); + + [[clang::analyze_as_method("optional(in_place_t, Args&&...)")]] + MyOpt(my_inplace_t, Args&&... args); +}; +``` + +## Signature Strings Are Opaque Keys + +The signature string is treated as a symbolic disambiguation key, **not** a type +expression to be resolved. The model compares the whole string +(e.g. `"optional(nullopt_t)"`) by equality against its own case labels. It does +not look up the target class, resolve `nullopt_t` (or `iterator`, `T`, …) to a +`QualType`, or inspect the target's real overloads. This matches how the rest of +the model already works: every case fires on a spelled name or an +`analyze_as_method` tag, plus a receiver/result type recognised as optional (via +`AnalyzeAsClassAttr`). + +The strings should still *read* like real target signatures +(`"insert(iterator, const T&)"`) — for author clarity, and so a future +validation pass could resolve them against the real target class — but nothing +resolves them today. Resolving types against the target class's actual overloads +(and diagnosing a signature that matches none) is a **later validation phase, not +abandoned** — general overload matching is fully delivered by the opaque-key +mechanism above (one distinct key → one case → one transfer function, for +arbitrarily many overloads) and does not depend on it. See "Backwards +Compatibility" below. + +## Matching Strategy + +When the model encounters an `analyze_as_method` attribute with a signature string: + +1. Retain the full string as the match key (the name/param split is used only for + Sema well-formedness validation, not for type resolution). +2. Register a `MatchSwitch` case whose matcher fires when a call or construct + carries that exact signature key *and* the receiver/result type is recognised + as optional. +3. Attach the transfer function for the intended behaviour to that case. + +Because `MatchSwitch` applies the first matching case, an attribute-driven case +that would otherwise be shadowed by a generic case (e.g. the value/conversion +constructor) must be registered **before** it. + +This is overload *identification by declared intent*, not overload *resolution*: +the author states which target overload they mean via the string, and the model +routes to the matching case — no candidate ranking or implicit conversions. + +## Genericity: not tied to std::optional + +The annotation-matching layer must work for **any** target class mapped via +`analyze_as_class` (`std::vector`, `std::unique_ptr`, …), not just `std::optional`. +Two layers, with a clean split: + +- **The matcher is target-class-agnostic.** `hasAnalyzeAsMethodName` compares the + attribute string against the model's query string; it never references + `std::optional`. Every target class reuses it unchanged. +- **Matching semantics — "accept either":** + - a *bare* query (`"emplace"`) matches an annotation by its **name part**, + whether or not the annotation carries a signature; + - a *signature* query (`"insert(iterator, const T&)"`) matches the **full + string** exactly. + + This lets annotations self-document with signatures while the model queries by + bare name for single-outcome operations and by full signature to disambiguate + multi-outcome overloads — the same rule (see "When a Signature Is Required") + applied uniformly across all target classes. +- **What stays target-specific:** the model's cases and transfer functions encode + one class's semantics (for this model, `std::optional`'s + emplace/reset/nullopt/…). Supporting a genuinely different class (e.g. + `std::vector`) means adding that class's cases and transfer functions — a + separate, larger effort — but the annotation-matching layer above is shared. + +**Consequence for emplace:** do *not* "fix" it by stripping its signature — that +is an `std::optional`-specific shortcut that fails for classes whose same-name +overloads differ in outcome. Instead keep the signature and make the matcher +accept a bare query against a param'd annotation via the name-part comparison. +That is the generic behaviour; std::optional's `emplace` is just its first +exercise. + +## Convenience Macro + +A macro can reduce boilerplate and avoid hand-writing signature strings: + +```cpp +#define ANALYZE_AS(method, ...) \ + [[clang::analyze_as_method(method "(" #__VA_ARGS__ ")")]] + +ANALYZE_AS("insert", iterator, const T&) +// expands to: [[clang::analyze_as_method("insert(iterator, const T&)")]] + +ANALYZE_AS("optional", nullopt_t) +// expands to: [[clang::analyze_as_method("optional(nullopt_t)")]] +``` + +## Backwards Compatibility + +A bare method name with no parentheses (e.g. `analyze_as_method("value")`) is +accepted **when the target name is outcome-unambiguous** — every overload the +model recognises under that name produces the same transfer outcome (`value`, +`emplace`, `reset`, `swap`, `has_value`, `value_or`). For these, a bare name and +a full signature are interchangeable. + +When a target name is **outcome-ambiguous** — different overloads produce +different outcomes (the constructor set: `optional(nullopt_t)` → empty vs +`optional(T&&)` / `optional(in_place_t, ...)` → engaged) — a full signature is +**required**, and a bare name for that name should be diagnosed. See "When a +Signature Is Required" below. + +## When a Signature Is Required + +Whether disambiguation is mandatory depends on the *outcomes* of the overloads +under a target name: + +- **Same outcome across overloads → optional.** If every overload the model maps + under a name produces the same analysis outcome, accept **either** the bare + name or a full signature — they're interchangeable. This is already the + behaviour of the name-only matcher, which compares only the pre-`(` name. +- **Differing outcomes → signature required.** If the overloads under a name + produce different outcomes, force disambiguation: each annotation must carry a + full signature, and a bare name for that name is an error. + +This holds identically for constructors. `optional` is outcome-ambiguous (empty +vs engaged), so every custom constructor mapping to it must use a full signature +(`optional(nullopt_t)`, `optional(T&&)`, `optional(in_place_t, Args&&...)`). + +**Source of truth.** "Do these overloads differ in outcome?" is answered by the +*model's own case/outcome table* — a finite, known vocabulary of which names map +to which transfer functions — not by resolving the real target class. So this +check needs no `#include` of the target and is independent of the deferred +real-target resolution. + +**Where enforced (open).** Because the outcome table lives in the model, Sema +(`isValidAnalyzeAsMethodAttr`) can only validate string *well-formedness*, not +outcome-ambiguity. The "signature required" diagnostic therefore belongs in the +model/check layer, or in a small shared registry that both matching and +validation consult — not in Sema. + +## Discarded Alternatives + +### Parameter count as a second attribute argument + +```cpp +[[clang::analyze_as_method("set", 0)]] // zero-arg overload +[[clang::analyze_as_method("set", 1)]] // one-arg overload +``` + +Discarded because it only works when overloads differ in arity. Same-arity overloads with different parameter types (e.g. `insert(iterator, const T&)` vs `insert(iterator, size_type)`) cannot be distinguished. Since type matching is needed for those cases anyway, parameter count is just a half-measure that delays the same work and results in two code paths instead of one. + +### Inferring the target overload from the client method's own signature + +```cpp +// Analyser would look at my_insert's params, map MyT→T / MyIter→iterator, +// and find the matching insert overload automatically. +[[clang::analyze_as_method("insert")]] +void my_insert(MyIter pos, const MyT& val); +``` + +Discarded because it constrains the client method to have parameters that map cleanly to the target method's signature. The whole purpose of annotations is that the custom class can have a completely different interface — different types, different argument order, extra parameters. Tying the two together defeats that goal. + +### Separate `analyze_as_constructor` attribute + +```cpp +[[clang::analyze_as_constructor("nullopt")]] +MyOpt(my_null_t); +``` + +Discarded in favour of the unified signature string approach. Constructors are just a special case of overloaded methods — they share a name and need disambiguation by parameter types. A separate attribute would duplicate the same disambiguation logic and add a second concept for users to learn. Using `analyze_as_method("optional(nullopt_t)")` handles constructors with the same mechanism as regular overloads. + +### Typestate model (`test_state("engaged/disengaged")`) + +```cpp +[[clang::analyze_test_state("engaged")]] +bool HasValue() const; +``` + +Discarded for the initial proposal. The typestate approach is more general and could support variant-like, resource, and container types with the same vocabulary. However, it adds significant complexity and is not needed for the current use case of mapping custom types to existing standard class interfaces. The simple string-comparison approach can land first; a typestate model could be layered on later if needed. diff --git a/hicketts/hicketts_optional.h b/hicketts/hicketts_optional.h new file mode 100644 index 0000000000000..9920039fec32a --- /dev/null +++ b/hicketts/hicketts_optional.h @@ -0,0 +1,109 @@ +#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_; } + + // Currently emits hicketts_optional.h:66:5: warning: 'clang::analyze_as_method' attribute argument not supported: + // 66 | [[clang::analyze_as_method("emplace(oops))")]] + [[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; } +}; + +} // namespace mylib + +#endif // HICKETTS_OPTIONAL_H_ diff --git a/hicketts/isValidAnalyzeAsMethodAttr.cpp b/hicketts/isValidAnalyzeAsMethodAttr.cpp new file mode 100644 index 0000000000000..02b8066539718 --- /dev/null +++ b/hicketts/isValidAnalyzeAsMethodAttr.cpp @@ -0,0 +1,53 @@ +static bool isValidAnalyzeAsMethodAttr(Decl *D, StringRef Tag) { + if (Tag.empty()) + return false; + + // Bare name with no parenthesised signature — accept as-is. + size_t OpenParen = Tag.find('('); + if (OpenParen == StringRef::npos) + return true; + + // Method name before '(' must be non-empty. + if (OpenParen == 0) + return false; + + // Must end with ')' — no trailing junk. + if (Tag.back() != ')') + return false; + + // Extract the contents between the outer '(' and ')'. + StringRef Params = Tag.slice(OpenParen + 1, Tag.size() - 1); + + // Empty param list "name()" is valid. + if (Params.empty()) + return true; + + // Walk the param list checking balanced/non-interleaved () and <> + // and that comma-separated segments are non-empty. + SmallVector<char, 4> Stack; + size_t SegStart = 0; + + for (size_t I = 0, E = Params.size(); I != E; ++I) { + char C = Params[I]; + if (C == '(' || C == '<') { + Stack.push_back(C); + } else if (C == ')') { + if (Stack.empty() || Stack.back() != '(') + return false; + Stack.pop_back(); + } else if (C == '>') { + if (Stack.empty() || Stack.back() != '<') + return false; + Stack.pop_back(); + } else if (C == ',' && Stack.empty()) { + if (Params.slice(SegStart, I).trim().empty()) + return false; + SegStart = I + 1; + } + } + + if (!Stack.empty()) + return false; + + return !Params.slice(SegStart, Params.size()).trim().empty(); +} diff --git a/hicketts/plan.md b/hicketts/plan.md new file mode 100644 index 0000000000000..e7b6f23051ca3 --- /dev/null +++ b/hicketts/plan.md @@ -0,0 +1,187 @@ +# Plan: Constructor Overload Disambiguation via Signature Strings (POC) + +## Why start with constructors + +Signature disambiguation only *earns its keep* when two overloads of the same +name need **different** transfer functions. For `emplace`, both overloads set +`has_value = true`, so disambiguating them changes nothing observable — a bad +place to start because you can't tell whether it works. + +Constructors are the opposite: `optional(nullopt_t)` → `has_value = false` but +`optional(T&&)` / `optional(in_place_t, ...)` → `true`. Same name, different +behaviour. Pick the wrong overload and the analysis is wrong — so there's a +clear pass/fail signal. + +Constructors are the *first* slice, not the whole feature. General overload +matching — disambiguating arbitrary overloaded methods on any target class — +stays the goal (see `constructors.md`). Regular overloaded methods are the very +next phase (Step 6), reusing the *same* signature-key mechanism; only the +transfer functions and the matched node kind (member call vs construct) differ. + +## How the model actually works (design decision this resolves) + +The model never looks up `std::optional`'s real declaration. Each case in the +`MatchSwitch` fires when a call/construct is spelled a known name **or** carries +an `analyze_as_method("X")` tag, *and* the receiver/result type is recognised as +optional (via `AnalyzeAsClassAttr`, see `hasOptionalClassName`). The attribute +string is a **symbolic disambiguation key** compared against the model's case +labels — not a query resolved against the target class's overloads. + +Consequence: the signature string (`"optional(nullopt_t)"`) is matched by string +equality to a case label. We do **not** resolve `nullopt_t` to a real QualType or +compare against `std::optional`'s actual constructor signatures. (This supersedes +the earlier "compare `QualType::getAsString()` against the target overload" +idea in `constructors.md` — that would be a foreign kind of lookup the model has +never needed, and it's blocked anyway because the test header doesn't +`#include <optional>`.) + +## Bare name vs required signature (design rule) + +Whether a full signature is *required* depends on the target name's outcomes: + +- **Same outcome across overloads → bare name or full signature both accepted.** + Names like `emplace`, `reset`, `swap`, `value`, `has_value`, `value_or` map + every overload to one outcome. The existing name-only matcher + (`hasAnalyzeAsMethodName`, which compares only the pre-`(` name) already accepts + either spelling — this half needs no new work. +- **Differing outcomes → full signature required, bare name is an error.** The + constructor name `optional` is the case: `optional(nullopt_t)` → empty vs + `optional(T&&)` / `optional(in_place_t, ...)` → engaged. Each custom + constructor must carry a full signature, and a bare `"optional"` should be + diagnosed. + +Source of truth: ambiguity is decided by the *model's* case/outcome table (a +finite, known vocabulary), not by resolving the real target class — so it needs +no `#include` and is independent of the deferred real-target resolution. + +Practically, each supported target name is either *name-matched* (unambiguous) or +*signature-matched* (ambiguous). Reifying that as a small registry +(name → [(signature, outcome)]) lets both the matcher and the "signature +required" check read from one place. Open: that diagnostic can't live in Sema +(which sees only the string, not outcomes) — it belongs in the model/check layer. + +## Current state of constructor handling + +The model already has three constructor cases (UncheckedOptionalAccessModel.cpp +`:288`–`:306`), but they match structurally by the **argument's real type name**, +not by attribute: + +- `isOptionalNulloptConstructor` — arg type is literally `std::nullopt_t` (or + absl/base/folly/bsl variants) → `has_value = false` +- `isOptionalInPlaceConstructor` — arg type is `in_place_t` → `true` +- `isOptionalValueOrConversionConstructor` — any other single-arg ctor → `true` + +**The bug this POC fixes:** a custom optional whose "empty" constructor takes a +differently-named tag type (`mylib::nothing_t`) is not recognised as the nullopt +constructor, so it falls through to the value/conversion case and is wrongly +marked engaged. Today this is a false negative: + +```cpp +mylib::HickettsOptional<int> x{mylib::nothing}; +x.unwrap(); // should warn (empty) — currently does NOT +``` + +## Goal (testable milestone) + +Tag the custom constructors and have the model route them to the correct +transfer function by signature: + +```cpp +[[clang::analyze_as_method("optional(nullopt_t)")]] HickettsOptional(nothing_t); // -> false +[[clang::analyze_as_method("optional(T&&)")]] HickettsOptional(T&&); // -> true +[[clang::analyze_as_method("optional(in_place_t, Args&&...)")]] HickettsOptional(my_inplace_t, Args&&...); // -> true +``` + +Success = `x{mylib::nothing}; x.unwrap();` now **warns**, while +`x{42}; x.unwrap();` stays quiet. + +## Steps + +1. **Sema validation** — already done (well-formedness of the signature string). + No further work needed for the POC. + +2. **Write the failing test first.** Add the tagged constructor(s) to + `hicketts_optional.h` and expectations to `test_hicketts_optional.cpp`, then + run clang-tidy and *watch the nullopt case fail to warn*. This grounds the + target and confirms the value/conversion case is what's (wrongly) firing. + +3. **Signature-aware matcher.** The existing `hasAnalyzeAsMethodName` (`:239`) + does `AttrValue.split('(').first`, so it strips params and cannot tell + `optional(nullopt_t)` from `optional(T&&)`. Add a matcher that compares the + **full** attribute string to a given signature label (e.g. + `hasAnalyzeAsSignature("optional(nullopt_t)")`). + +4. **Attribute-driven constructor cases.** Mirror the three structural cases, + but gate them on the signature key instead of the arg's type name: + - `"optional(nullopt_t)"` → `setHasValue(false)` + - `"optional(T&&)"` and `"optional(in_place_t, Args&&...)"` → `setHasValue(true)` + - a bare `"optional"` tag (no signature) → diagnose "signature required" per + the design rule above; do not silently route it to a constructor case + + **Ordering matters:** `MatchSwitch` applies the first matching case, so the + attribute-driven nullopt case must be registered *before* + `isOptionalValueOrConversionConstructor` — otherwise the generic value case + greedily matches first and the nullopt tag never gets a chance. + +5. **Verify.** Build clang-tidy, run it on the fixture. The nullopt-constructed + `unwrap()` should now warn; the value-constructed one should not. + +6. **Phase 2 — regular overloaded methods.** Apply the same full-signature + matcher to the member-call path (`CXXMemberCallExpr`), so overloaded *methods* + are disambiguated exactly like constructors — this is the general + overload-matching capability, not a throwaway. Concretely, tag the two + `emplace` overloads with distinct keys (`"emplace(Args&&...)"` vs + `"emplace(initializer_list<U>, Args&&...)"`) and confirm both route to their + case. + + Honesty about observability: within `std::optional` both `emplace` overloads + set `has_value = true`, so a *behavioural* difference between two same-named + method overloads only shows up with a richer target class (e.g. + `vector::insert` variants, or a custom `set()`-resets vs `set(T)`-assigns + pair). The matching *mechanism* is identical and is proven here; the richer + behavioural cases land when `analyze_as_class` is generalised beyond + `std::optional`. + +## Deferred (future phase, not abandoned) + +Resolving/validating a signature string against the target class's **real** +overloads — looking up `std::optional`/`std::vector` in the AST, pulling its +actual `QualType`s, and diagnosing a signature that names no real overload. This +is purely a *validation/safety* layer; overload matching itself does not need it. +It requires the target class to be present in the translation unit (an +`#include`) plus an AST lookup the model doesn't currently do, so it's a later +phase — the matching feature is complete without it. + +**TODO (spike, after the POC): scope the effort for real param validation.** +Investigate — don't implement — how much work real parameter validation would be, +and produce a short write-up with a rough estimate. Questions to answer: +- How to obtain the target decl: require an `#include` of the target header, or + look it up by qualified name via `ASTContext` / `Sema` lookup? +- Where it runs: Sema attribute handling vs the model/check layer. +- Type-spelling mismatches: canonicalisation, template params (`T`, `Args&&...`), + sugar/aliases — how strict must the comparison be, and via what API + (`QualType::getCanonicalType`, `getAsString` with a `PrintingPolicy`, …)? +- What diagnostics to emit (signature names no real overload / ambiguous) and + their wording. +Deliverable: a paragraph or two estimating scope, not code. + +## Files to modify + +1. `clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp` + — new full-signature matcher + attribute-driven constructor cases (ordered + before the generic value/conversion case) +2. `hicketts/hicketts_optional.h` — tagged constructors + `my_inplace_t` tag type +3. `hicketts/test_hicketts_optional.cpp` — nullopt-vs-value construction cases +4. (Later) unit + clang-tidy integration tests in the real tree +5. Sema — already done + +## Verification + +1. Build: `cmake --build build-llvm --target clang-tidy` +2. Fast loop on the fixture (from `hicketts/`): + ``` + clang-tidy -checks='bugprone-unchecked-optional-access' \ + test_hicketts_optional.cpp -- -I . -Wno-undefined-inline + ``` +3. Later: unit test binary for `UncheckedOptionalAccessModelTest` and `llvm-lit` + on the `unchecked-optional-access` integration test. diff --git a/hicketts/test_hicketts_optional.cpp b/hicketts/test_hicketts_optional.cpp new file mode 100644 index 0000000000000..04a4526d94409 --- /dev/null +++ b/hicketts/test_hicketts_optional.cpp @@ -0,0 +1,108 @@ +// 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.cpp -- -I . -std=c++17 -Wno-undefined-inline + +#include "hicketts_optional.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; +} diff --git a/warnings.log b/warnings.log new file mode 100644 index 0000000000000..aae7584deb00f --- /dev/null +++ b/warnings.log @@ -0,0 +1,18 @@ +15 warnings generated. +/Users/khicketts/github_com/llvm-project/hicketts_test/test_hicketts_optional.cpp:13:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + 13 | Val.unwrap(); // unchecked access — may be empty + | ^~~ +/Users/khicketts/github_com/llvm-project/hicketts_test/test_hicketts_optional.cpp:17:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + 17 | Val.value(); // unchecked access — may be empty + | ^~~ +/Users/khicketts/github_com/llvm-project/hicketts_test/test_hicketts_optional.cpp:21:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + 21 | Val.deref(); // unchecked access — may be empty + | ^~~ +/Users/khicketts/github_com/llvm-project/hicketts_test/test_hicketts_optional.cpp:60:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + 60 | Val.unwrap(); // unsafe — value was cleared + | ^~~ +/Users/khicketts/github_com/llvm-project/hicketts_test/test_hicketts_optional.cpp:67:5: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + 67 | A.unwrap(); // unsafe — a's state is now unknown + | ^ +Suppressed 10 warnings (10 in non-user code). +Use -header-filter=.* or leave it as default to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
