https://github.com/StoeckOverflow updated https://github.com/llvm/llvm-project/pull/209408
>From 91dbf3cd44e960b2568724e0c9bf4990d7684040 Mon Sep 17 00:00:00 2001 From: stoeckoverflow <[email protected]> Date: Fri, 19 Jun 2026 12:57:27 +0200 Subject: [PATCH 1/3] [APINotes] Diagnose invalid Where.Parameters selectors --- clang/include/clang/APINotes/APINotesReader.h | 12 ++ clang/lib/APINotes/APINotesReader.cpp | 79 +++++++++ clang/lib/APINotes/APINotesYAMLCompiler.cpp | 71 +++++++- clang/lib/Sema/SemaAPINotes.cpp | 155 ++++++++++++++++++ .../WhereParametersDiagnostics.apinotes | 50 ++++++ .../Headers/WhereParametersDiagnostics.h | 18 ++ .../APINotes/Inputs/Headers/module.modulemap | 5 + .../APINotes.apinotes | 66 ++++++++ .../WhereParametersDiagnostics.h | 16 ++ .../APINotes/where-parameters-diagnostics.cpp | 41 +++++ 10 files changed, 511 insertions(+), 2 deletions(-) create mode 100644 clang/test/APINotes/Inputs/Headers/WhereParametersDiagnostics.apinotes create mode 100644 clang/test/APINotes/Inputs/Headers/WhereParametersDiagnostics.h create mode 100644 clang/test/APINotes/Inputs/WhereParametersDuplicateSelectorDiag/APINotes.apinotes create mode 100644 clang/test/APINotes/Inputs/WhereParametersDuplicateSelectorDiag/WhereParametersDiagnostics.h create mode 100644 clang/test/APINotes/where-parameters-diagnostics.cpp diff --git a/clang/include/clang/APINotes/APINotesReader.h b/clang/include/clang/APINotes/APINotesReader.h index 761745e20b61a..640be8ad5acb3 100644 --- a/clang/include/clang/APINotes/APINotesReader.h +++ b/clang/include/clang/APINotes/APINotesReader.h @@ -17,6 +17,7 @@ #include "clang/APINotes/Types.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/Support/Error.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VersionTuple.h" @@ -169,6 +170,11 @@ class APINotesReader { lookupCXXMethod(ContextID CtxID, llvm::StringRef Name, llvm::ArrayRef<std::string> Parameters); + /// Collect exact parameter selectors stored for the given C++ method. + void collectCXXMethodParameterSelectors( + ContextID CtxID, llvm::StringRef Name, + llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors); + /// Look for information regarding the given global variable. /// /// \param Name The name of the global variable. @@ -195,6 +201,12 @@ class APINotesReader { llvm::ArrayRef<std::string> Parameters, std::optional<Context> Ctx = std::nullopt); + /// Collect exact parameter selectors stored for the given global function. + void collectGlobalFunctionParameterSelectors( + llvm::StringRef Name, + llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors, + std::optional<Context> Ctx = std::nullopt); + /// Look for information regarding the given enumerator. /// /// \param Name The name of the enumerator. diff --git a/clang/lib/APINotes/APINotesReader.cpp b/clang/lib/APINotes/APINotesReader.cpp index 888844fbb6cd9..944beccb9d2df 100644 --- a/clang/lib/APINotes/APINotesReader.cpp +++ b/clang/lib/APINotes/APINotesReader.cpp @@ -19,6 +19,8 @@ #include "llvm/Bitstream/BitstreamReader.h" #include "llvm/Support/DJB.h" #include "llvm/Support/OnDiskHashTable.h" +#include <string> +#include <utility> namespace clang { namespace api_notes { @@ -833,6 +835,16 @@ class APINotesReader::Implementation { /// optional if the string is unknown. std::optional<IdentifierID> getIdentifier(llvm::StringRef Str); + /// Retrieve the identifier string for the given ID, or an empty optional if + /// the ID is unknown. + std::optional<llvm::StringRef> getIdentifierString(IdentifierID ID); + + /// Collect exact parameter selectors stored in the given function-like table. + template <typename TableT> + void collectFunctionParameterSelectors( + TableT *Table, uint32_t ParentContextID, llvm::StringRef Name, + llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors); + /// Retrieve the selector ID for the given selector, or an empty /// optional if the string is unknown. std::optional<SelectorID> getSelector(ObjCSelectorRef Selector); @@ -893,6 +905,56 @@ APINotesReader::Implementation::getIdentifier(llvm::StringRef Str) { return *Known; } +std::optional<llvm::StringRef> +APINotesReader::Implementation::getIdentifierString(IdentifierID ID) { + if (!IdentifierTable) + return std::nullopt; + + if (ID == IdentifierID(0)) + return llvm::StringRef(); + + for (llvm::StringRef Identifier : IdentifierTable->keys()) { + auto KnownID = IdentifierTable->find(Identifier); + if (KnownID != IdentifierTable->end() && *KnownID == ID) + return Identifier; + } + + return std::nullopt; +} + +template <typename TableT> +void APINotesReader::Implementation::collectFunctionParameterSelectors( + TableT *Table, uint32_t ParentContextID, llvm::StringRef Name, + llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors) { + if (!Table) + return; + + std::optional<IdentifierID> NameID = getIdentifier(Name); + if (!NameID) + return; + + for (auto I = Table->key_begin(), E = Table->key_end(); I != E; ++I) { + FunctionTableKey Key = I.getInternalKey(); + if (Key.parentContextID != ParentContextID || + Key.nameID != static_cast<unsigned>(*NameID) || !Key.parameterTypeIDs) + continue; + + llvm::SmallVector<std::string, 4> ParameterSelector; + ParameterSelector.reserve(Key.parameterTypeIDs->size()); + bool Failed = false; + for (IdentifierID TypeID : *Key.parameterTypeIDs) { + std::optional<llvm::StringRef> TypeName = getIdentifierString(TypeID); + if (!TypeName) { + Failed = true; + break; + } + ParameterSelector.push_back(TypeName->str()); + } + if (!Failed) + Selectors.push_back(std::move(ParameterSelector)); + } +} + std::optional<FunctionTableKey> APINotesReader::Implementation::getFunctionKey(uint32_t ParentContextID, llvm::StringRef Name) { @@ -2351,6 +2413,13 @@ auto APINotesReader::lookupCXXMethod(ContextID CtxID, llvm::StringRef Name, return lookupCXXMethodImpl(CtxID, Name, Parameters); } +void APINotesReader::collectCXXMethodParameterSelectors( + ContextID CtxID, llvm::StringRef Name, + llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors) { + Implementation->collectFunctionParameterSelectors( + Implementation->CXXMethodTable.get(), CtxID.Value, Name, Selectors); +} + auto APINotesReader::lookupCXXMethodImpl(ContextID CtxID, llvm::StringRef Name) -> VersionedInfo<CXXMethodInfo> { if (!Implementation->CXXMethodTable) @@ -2418,6 +2487,16 @@ auto APINotesReader::lookupGlobalFunction( return lookupGlobalFunctionImpl(Name, Parameters, Ctx); } +void APINotesReader::collectGlobalFunctionParameterSelectors( + llvm::StringRef Name, + llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors, + std::optional<Context> Ctx) { + uint32_t ParentContextID = Ctx ? Ctx->id.Value : static_cast<uint32_t>(-1); + Implementation->collectFunctionParameterSelectors( + Implementation->GlobalFunctionTable.get(), ParentContextID, Name, + Selectors); +} + auto APINotesReader::lookupGlobalFunctionImpl(llvm::StringRef Name, std::optional<Context> Ctx) -> VersionedInfo<GlobalFunctionInfo> { diff --git a/clang/lib/APINotes/APINotesYAMLCompiler.cpp b/clang/lib/APINotes/APINotesYAMLCompiler.cpp index 1c3a59873db25..058f9da338661 100644 --- a/clang/lib/APINotes/APINotesYAMLCompiler.cpp +++ b/clang/lib/APINotes/APINotesYAMLCompiler.cpp @@ -18,6 +18,7 @@ #include "clang/APINotes/Types.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/Specifiers.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/VersionTuple.h" @@ -788,6 +789,47 @@ bool clang::api_notes::parseAndDumpAPINotes(StringRef YI, namespace { using namespace api_notes; +struct KnownFunctionSelector { + llvm::StringRef Name; + llvm::ArrayRef<llvm::StringRef> Parameters; +}; + +static bool equalParameterSelectors(llvm::ArrayRef<llvm::StringRef> LHS, + llvm::ArrayRef<llvm::StringRef> RHS) { + if (LHS.size() != RHS.size()) + return false; + + for (unsigned I = 0, E = LHS.size(); I != E; ++I) + if (LHS[I] != RHS[I]) + return false; + + return true; +} + +static bool +hasFunctionSelector(llvm::ArrayRef<KnownFunctionSelector> KnownSelectors, + llvm::StringRef Name, + llvm::ArrayRef<llvm::StringRef> Parameters) { + for (const KnownFunctionSelector &Known : KnownSelectors) + if (Known.Name == Name && + equalParameterSelectors(Known.Parameters, Parameters)) + return true; + + return false; +} + +static std::string +formatWhereParameters(llvm::ArrayRef<llvm::StringRef> Parameters) { + std::string Result = "["; + for (unsigned I = 0, E = Parameters.size(); I != E; ++I) { + if (I) + Result += ", "; + Result += Parameters[I]; + } + Result += "]"; + return Result; +} + class YAMLConverter { const Module &M; APINotesWriter Writer; @@ -1162,11 +1204,24 @@ class YAMLConverter { Writer.addField(TagCtxID, Field.Name, FI, SwiftVersion); } + llvm::SmallVector<KnownFunctionSelector, 4> KnownMethodSelectors; for (const auto &CXXMethod : T.Methods) { auto WhereParameters = getWhereParameters(CXXMethod); if (!WhereParameters.first) continue; + if (WhereParameters.second) { + if (hasFunctionSelector(KnownMethodSelectors, CXXMethod.Name, + *WhereParameters.second)) { + emitError(llvm::Twine("duplicate definition of C++ method '") + + CXXMethod.Name + "' with Where.Parameters " + + formatWhereParameters(*WhereParameters.second)); + continue; + } + KnownMethodSelectors.push_back( + {CXXMethod.Name, *WhereParameters.second}); + } + CXXMethodInfo MI; convertFunction(CXXMethod, MI); if (WhereParameters.second) @@ -1243,13 +1298,25 @@ class YAMLConverter { // Write all global functions. llvm::StringSet<> KnownNameOnlyFunctions; + llvm::SmallVector<KnownFunctionSelector, 4> KnownFunctionSelectors; for (const auto &Function : TLItems.Functions) { auto WhereParameters = getWhereParameters(Function); if (!WhereParameters.first) continue; - // Check for duplicate name-only global functions. Selector-aware - // duplicate diagnostics are handled by a later overload-matching PR. + if (WhereParameters.second) { + if (hasFunctionSelector(KnownFunctionSelectors, Function.Name, + *WhereParameters.second)) { + emitError(llvm::Twine("duplicate definition of global function '") + + Function.Name + "' with Where.Parameters " + + formatWhereParameters(*WhereParameters.second)); + continue; + } + KnownFunctionSelectors.push_back( + {Function.Name, *WhereParameters.second}); + } + + // Check for duplicate name-only global functions. if (!WhereParameters.second && !KnownNameOnlyFunctions.insert(Function.Name).second) { emitError(llvm::Twine("multiple definitions of global function '") + diff --git a/clang/lib/Sema/SemaAPINotes.cpp b/clang/lib/Sema/SemaAPINotes.cpp index 1ab1eac4a5434..e9e90c93755d3 100644 --- a/clang/lib/Sema/SemaAPINotes.cpp +++ b/clang/lib/Sema/SemaAPINotes.cpp @@ -1017,6 +1017,50 @@ struct APINotesParameterSelectorCandidates { std::optional<APINotesParameterSelector> Desugared; }; +struct APINotesParameterSelectorSet { + SmallVector<APINotesParameterSelector, 4> Selectors; + + void add(const APINotesParameterSelector &Selector) { + Selectors.push_back(Selector); + } + + void add(ArrayRef<std::string> Parameters) { + APINotesParameterSelector Selector; + Selector.Parameters.append(Parameters.begin(), Parameters.end()); + add(Selector); + } + + void add(const APINotesParameterSelectorCandidates &Candidates) { + add(Candidates.Source); + if (Candidates.Desugared) + add(*Candidates.Desugared); + } + + bool contains(ArrayRef<std::string> Parameters) const { + for (const APINotesParameterSelector &Selector : Selectors) { + if (Selector.Parameters.size() != Parameters.size()) + continue; + + bool Matches = true; + for (unsigned I = 0, E = Parameters.size(); I != E; ++I) { + if (Selector.Parameters[I] == Parameters[I]) + continue; + Matches = false; + break; + } + if (Matches) + return true; + } + + return false; + } + + bool empty() const { return Selectors.empty(); } + + auto begin() const { return Selectors.begin(); } + auto end() const { return Selectors.end(); } +}; + static PrintingPolicy getAPINotesParameterSelectorPrintingPolicy(const ASTContext &Context) { PrintingPolicy Policy(Context.getLangOpts()); @@ -1072,6 +1116,84 @@ getAPINotesParameterSelectorCandidates(const Sema &S, const FunctionDecl *FD) { return Candidates; } +static APINotesParameterSelectorSet +makeParameterSelectorSet(ArrayRef<SmallVector<std::string, 4>> Selectors) { + APINotesParameterSelectorSet Set; + for (const SmallVector<std::string, 4> &Selector : Selectors) + Set.add(Selector); + return Set; +} + +static std::string +formatParameterSelectorForDiagnostic(ArrayRef<std::string> Parameters) { + std::string Result = "["; + for (unsigned I = 0, E = Parameters.size(); I != E; ++I) { + if (I) + Result += ", "; + Result += Parameters[I]; + } + Result += "]"; + return Result; +} + +static void collectOverloadParameterSelectors(const Sema &S, + const FunctionDecl *FD, + APINotesParameterSelectorSet &Set, + bool &IsRepresentative) { + IsRepresentative = false; + bool FoundRepresentative = false; + + auto AddCandidate = [&](const FunctionDecl *Candidate) { + if (!FoundRepresentative) { + IsRepresentative = + Candidate->getCanonicalDecl() == FD->getCanonicalDecl(); + FoundRepresentative = true; + } + + if (auto Candidates = getAPINotesParameterSelectorCandidates(S, Candidate)) + Set.add(*Candidates); + }; + + for (NamedDecl *ND : FD->getDeclContext()->lookup(FD->getDeclName())) { + if (auto *Candidate = dyn_cast<FunctionDecl>(ND)) + AddCandidate(Candidate); + } + + if (FoundRepresentative) + return; + + for (Decl *D : FD->getDeclContext()->decls()) { + auto *ND = dyn_cast<NamedDecl>(D); + if (!ND || ND->getDeclName() != FD->getDeclName()) + continue; + + if (auto *Candidate = dyn_cast<FunctionDecl>(ND)) + AddCandidate(Candidate); + } + + if (!FoundRepresentative) + AddCandidate(FD); +} + +static void diagnoseUnmatchedParameterSelectors( + Sema &S, SourceLocation Loc, StringRef Name, + const APINotesParameterSelectorSet &APINotesSelectors, + const APINotesParameterSelectorSet &DeclarationSelectors) { + if (DeclarationSelectors.empty()) + return; + + for (const APINotesParameterSelector &APINotesSelector : APINotesSelectors) { + if (DeclarationSelectors.contains(APINotesSelector.Parameters)) + continue; + + S.Diag(Loc, diag::warn_apinotes_message) + << (llvm::Twine("API notes entry for '") + Name + + "' has unmatched Where.Parameters " + + formatParameterSelectorForDiagnostic(APINotesSelector.Parameters)) + .str(); + } +} + // Apply the first exact selector entry found. This preserves source-spelling // precedence over the desugared fallback and avoids applying multiple exact // entries for the same declaration. @@ -1131,6 +1253,12 @@ void Sema::ProcessAPINotes(Decl *D) { if (FD->getDeclName().isIdentifier()) { auto ParameterSelectorCandidates = getAPINotesParameterSelectorCandidates(*this, FD); + + APINotesParameterSelectorSet DeclarationSelectors; + bool DiagnoseUnmatchedSelectors = false; + collectOverloadParameterSelectors(*this, FD, DeclarationSelectors, + DiagnoseUnmatchedSelectors); + for (auto Reader : Readers) { auto Info = Reader->lookupGlobalFunction(FD->getName(), APINotesContext); @@ -1143,6 +1271,17 @@ void Sema::ProcessAPINotes(Decl *D) { return Reader->lookupGlobalFunction(FD->getName(), Parameters, APINotesContext); }); + + if (DiagnoseUnmatchedSelectors && !DeclarationSelectors.empty()) { + SmallVector<SmallVector<std::string, 4>, 4> RawAPINotesSelectors; + Reader->collectGlobalFunctionParameterSelectors( + FD->getName(), RawAPINotesSelectors, APINotesContext); + APINotesParameterSelectorSet APINotesSelectors = + makeParameterSelectorSet(RawAPINotesSelectors); + diagnoseUnmatchedParameterSelectors( + *this, FD->getLocation(), FD->getName(), APINotesSelectors, + DeclarationSelectors); + } } } @@ -1348,6 +1487,22 @@ void Sema::ProcessAPINotes(Decl *D) { return Reader->lookupCXXMethod(Context->id, MethodName, Parameters); }); + + APINotesParameterSelectorSet DeclarationSelectors; + bool DiagnoseUnmatchedSelectors = false; + collectOverloadParameterSelectors(*this, CXXMethod, + DeclarationSelectors, + DiagnoseUnmatchedSelectors); + if (DiagnoseUnmatchedSelectors && !DeclarationSelectors.empty()) { + SmallVector<SmallVector<std::string, 4>, 4> RawAPINotesSelectors; + Reader->collectCXXMethodParameterSelectors( + Context->id, MethodName, RawAPINotesSelectors); + APINotesParameterSelectorSet APINotesSelectors = + makeParameterSelectorSet(RawAPINotesSelectors); + diagnoseUnmatchedParameterSelectors( + *this, CXXMethod->getLocation(), MethodName, + APINotesSelectors, DeclarationSelectors); + } } } } diff --git a/clang/test/APINotes/Inputs/Headers/WhereParametersDiagnostics.apinotes b/clang/test/APINotes/Inputs/Headers/WhereParametersDiagnostics.apinotes new file mode 100644 index 0000000000000..2cf62c2b5e6be --- /dev/null +++ b/clang/test/APINotes/Inputs/Headers/WhereParametersDiagnostics.apinotes @@ -0,0 +1,50 @@ +--- +Name: WhereParametersDiagnostics +Functions: +- Name: unmatchedGlobal + Where: + Parameters: + - int + SwiftName: shouldNotApplyGlobal(_:) +- Name: diagnosticBroadGlobal + SwiftPrivate: true +- Name: diagnosticBroadGlobal + Where: + Parameters: + - int + SwiftName: shouldNotApplyBroadGlobal(_:) +- Name: diagnosticMatchedGlobal + Where: + Parameters: + - int + SwiftName: diagnosticMatchedGlobal(_:) +- Name: diagnosticAliasMatchedGlobal + Where: + Parameters: + - int + SwiftName: diagnosticAliasMatchedGlobal(_:) +Tags: +- Name: DiagnosticWidget + Methods: + - Name: unmatchedMethod + Where: + Parameters: + - int + SwiftName: shouldNotApplyMethod(_:) + - Name: diagnosticBroadMethod + SwiftPrivate: true + - Name: diagnosticBroadMethod + Where: + Parameters: + - int + SwiftName: shouldNotApplyBroadMethod(_:) + - Name: diagnosticMatchedMethod + Where: + Parameters: + - int + SwiftName: diagnosticMatchedMethod(_:) + - Name: diagnosticAliasMatchedMethod + Where: + Parameters: + - int + SwiftName: diagnosticAliasMatchedMethod(_:) diff --git a/clang/test/APINotes/Inputs/Headers/WhereParametersDiagnostics.h b/clang/test/APINotes/Inputs/Headers/WhereParametersDiagnostics.h new file mode 100644 index 0000000000000..c39ccd57541aa --- /dev/null +++ b/clang/test/APINotes/Inputs/Headers/WhereParametersDiagnostics.h @@ -0,0 +1,18 @@ +#ifndef WHERE_PARAMETERS_DIAGNOSTICS_H +#define WHERE_PARAMETERS_DIAGNOSTICS_H + +using DiagnosticAliasInt = int; + +void unmatchedGlobal(float); +void diagnosticBroadGlobal(float); +void diagnosticMatchedGlobal(int); +void diagnosticAliasMatchedGlobal(DiagnosticAliasInt); + +struct DiagnosticWidget { + void unmatchedMethod(float); + void diagnosticBroadMethod(float); + void diagnosticMatchedMethod(int); + void diagnosticAliasMatchedMethod(DiagnosticAliasInt); +}; + +#endif // WHERE_PARAMETERS_DIAGNOSTICS_H diff --git a/clang/test/APINotes/Inputs/Headers/module.modulemap b/clang/test/APINotes/Inputs/Headers/module.modulemap index 592d482ea7a57..d00b96c55b839 100644 --- a/clang/test/APINotes/Inputs/Headers/module.modulemap +++ b/clang/test/APINotes/Inputs/Headers/module.modulemap @@ -75,3 +75,8 @@ module WhereParametersSema { header "WhereParametersSema.h" export * } + +module WhereParametersDiagnostics { + header "WhereParametersDiagnostics.h" + export * +} diff --git a/clang/test/APINotes/Inputs/WhereParametersDuplicateSelectorDiag/APINotes.apinotes b/clang/test/APINotes/Inputs/WhereParametersDuplicateSelectorDiag/APINotes.apinotes new file mode 100644 index 0000000000000..d99ea9c0f4ce9 --- /dev/null +++ b/clang/test/APINotes/Inputs/WhereParametersDuplicateSelectorDiag/APINotes.apinotes @@ -0,0 +1,66 @@ +--- +Name: WhereParametersDiagnostics +Functions: +- Name: duplicateGlobal + Where: + Parameters: + - int + SwiftName: duplicateGlobalA(_:) +- Name: duplicateGlobal + Where: + Parameters: + - int + SwiftName: duplicateGlobalB(_:) +- Name: duplicateEmpty + Where: + Parameters: [] + SwiftName: duplicateEmptyA() +- Name: duplicateEmpty + Where: + Parameters: [] + SwiftName: duplicateEmptyB() +- Name: allowedGlobal + SwiftPrivate: true +- Name: allowedGlobal + Where: + Parameters: + - int + SwiftName: allowedGlobalInt(_:) +- Name: allowedGlobal + Where: + Parameters: + - double + SwiftName: allowedGlobalDouble(_:) +Tags: +- Name: DiagnosticWidget + Methods: + - Name: duplicateMethod + Where: + Parameters: + - int + SwiftName: duplicateMethodA(_:) + - Name: duplicateMethod + Where: + Parameters: + - int + SwiftName: duplicateMethodB(_:) + - Name: duplicateEmpty + Where: + Parameters: [] + SwiftName: duplicateEmptyA() + - Name: duplicateEmpty + Where: + Parameters: [] + SwiftName: duplicateEmptyB() + - Name: allowed + SwiftPrivate: true + - Name: allowed + Where: + Parameters: + - int + SwiftName: allowedInt(_:) + - Name: allowed + Where: + Parameters: + - double + SwiftName: allowedDouble(_:) diff --git a/clang/test/APINotes/Inputs/WhereParametersDuplicateSelectorDiag/WhereParametersDiagnostics.h b/clang/test/APINotes/Inputs/WhereParametersDuplicateSelectorDiag/WhereParametersDiagnostics.h new file mode 100644 index 0000000000000..406a848a04b03 --- /dev/null +++ b/clang/test/APINotes/Inputs/WhereParametersDuplicateSelectorDiag/WhereParametersDiagnostics.h @@ -0,0 +1,16 @@ +#ifndef WHERE_PARAMETERS_DIAGNOSTICS_H +#define WHERE_PARAMETERS_DIAGNOSTICS_H + +void duplicateGlobal(int); +void duplicateEmpty(); +void allowedGlobal(int); +void allowedGlobal(double); + +struct DiagnosticWidget { + void duplicateMethod(int); + void duplicateEmpty(); + void allowed(int); + void allowed(double); +}; + +#endif // WHERE_PARAMETERS_DIAGNOSTICS_H diff --git a/clang/test/APINotes/where-parameters-diagnostics.cpp b/clang/test/APINotes/where-parameters-diagnostics.cpp new file mode 100644 index 0000000000000..7bb033973f728 --- /dev/null +++ b/clang/test/APINotes/where-parameters-diagnostics.cpp @@ -0,0 +1,41 @@ +// RUN: not %clang_cc1 -fsyntax-only -fapinotes %s -I %S/Inputs/WhereParametersDuplicateSelectorDiag 2>&1 | FileCheck %s --check-prefix=DUPLICATE +// RUN: rm -rf %t && mkdir -p %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/WhereParametersDiagnostics -fdisable-module-hash -fapinotes-modules -Wapinotes -fsyntax-only -I %S/Inputs/Headers %s -x c++ 2>&1 | FileCheck %s --check-prefix=UNMATCHED --implicit-check-not=diagnosticMatchedGlobal --implicit-check-not=diagnosticAliasMatchedGlobal --implicit-check-not=diagnosticMatchedMethod --implicit-check-not=diagnosticAliasMatchedMethod +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/WhereParametersDiagnostics -fdisable-module-hash -fapinotes-modules -Wno-apinotes -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter diagnosticBroadGlobal -x c++ | FileCheck %s --check-prefix=BROAD-GLOBAL +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/WhereParametersDiagnostics -fdisable-module-hash -fapinotes-modules -Wno-apinotes -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter DiagnosticWidget::diagnosticBroadMethod -x c++ | FileCheck %s --check-prefix=BROAD-METHOD +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/WhereParametersDiagnostics -fdisable-module-hash -fapinotes-modules -Wno-apinotes -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter diagnosticMatchedGlobal -x c++ | FileCheck %s --check-prefix=MATCHED-GLOBAL +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/WhereParametersDiagnostics -fdisable-module-hash -fapinotes-modules -Wno-apinotes -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter diagnosticAliasMatchedGlobal -x c++ | FileCheck %s --check-prefix=ALIAS-MATCHED-GLOBAL +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/WhereParametersDiagnostics -fdisable-module-hash -fapinotes-modules -Wno-apinotes -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter DiagnosticWidget::diagnosticMatchedMethod -x c++ | FileCheck %s --check-prefix=MATCHED-METHOD +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/WhereParametersDiagnostics -fdisable-module-hash -fapinotes-modules -Wno-apinotes -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter DiagnosticWidget::diagnosticAliasMatchedMethod -x c++ | FileCheck %s --check-prefix=ALIAS-MATCHED-METHOD + +#include "WhereParametersDiagnostics.h" + +// DUPLICATE: error: duplicate definition of global function 'duplicateGlobal' with Where.Parameters [int] +// DUPLICATE: error: duplicate definition of global function 'duplicateEmpty' with Where.Parameters [] +// DUPLICATE: error: duplicate definition of C++ method 'duplicateMethod' with Where.Parameters [int] +// DUPLICATE: error: duplicate definition of C++ method 'duplicateEmpty' with Where.Parameters [] + +// UNMATCHED-DAG: warning: API notes entry for 'unmatchedGlobal' has unmatched Where.Parameters [int] +// UNMATCHED-DAG: warning: API notes entry for 'diagnosticBroadGlobal' has unmatched Where.Parameters [int] +// UNMATCHED-DAG: warning: API notes entry for 'unmatchedMethod' has unmatched Where.Parameters [int] +// UNMATCHED-DAG: warning: API notes entry for 'diagnosticBroadMethod' has unmatched Where.Parameters [int] + +// BROAD-GLOBAL: FunctionDecl {{.+}} diagnosticBroadGlobal 'void (float)' +// BROAD-GLOBAL: SwiftPrivateAttr +// BROAD-GLOBAL-NOT: SwiftNameAttr + +// BROAD-METHOD: CXXMethodDecl {{.+}} diagnosticBroadMethod 'void (float)' +// BROAD-METHOD: SwiftPrivateAttr +// BROAD-METHOD-NOT: SwiftNameAttr + +// MATCHED-GLOBAL: FunctionDecl {{.+}} diagnosticMatchedGlobal 'void (int)' +// MATCHED-GLOBAL: SwiftNameAttr {{.+}} "diagnosticMatchedGlobal(_:)" + +// ALIAS-MATCHED-GLOBAL: FunctionDecl {{.+}} diagnosticAliasMatchedGlobal 'void (DiagnosticAliasInt)' +// ALIAS-MATCHED-GLOBAL: SwiftNameAttr {{.+}} "diagnosticAliasMatchedGlobal(_:)" + +// MATCHED-METHOD: CXXMethodDecl {{.+}} diagnosticMatchedMethod 'void (int)' +// MATCHED-METHOD: SwiftNameAttr {{.+}} "diagnosticMatchedMethod(_:)" + +// ALIAS-MATCHED-METHOD: CXXMethodDecl {{.+}} diagnosticAliasMatchedMethod 'void (DiagnosticAliasInt)' +// ALIAS-MATCHED-METHOD: SwiftNameAttr {{.+}} "diagnosticAliasMatchedMethod(_:)" >From a5b094c24c74322f598481359075d0469aa1c068 Mon Sep 17 00:00:00 2001 From: stoeckoverflow <[email protected]> Date: Tue, 14 Jul 2026 14:05:04 +0200 Subject: [PATCH 2/3] [APINotes] Make Where.Parameters diagnostics non-loading and and address reviewer feedback --- clang/include/clang/APINotes/APINotesReader.h | 4 +- clang/include/clang/APINotes/Types.h | 7 +++ clang/lib/APINotes/APINotesReader.cpp | 31 +++++----- clang/lib/APINotes/APINotesTypes.cpp | 25 ++++++++ clang/lib/APINotes/APINotesYAMLCompiler.cpp | 31 +--------- clang/lib/Sema/SemaAPINotes.cpp | 60 +++++++------------ 6 files changed, 72 insertions(+), 86 deletions(-) diff --git a/clang/include/clang/APINotes/APINotesReader.h b/clang/include/clang/APINotes/APINotesReader.h index 640be8ad5acb3..1289c1366437c 100644 --- a/clang/include/clang/APINotes/APINotesReader.h +++ b/clang/include/clang/APINotes/APINotesReader.h @@ -171,7 +171,7 @@ class APINotesReader { llvm::ArrayRef<std::string> Parameters); /// Collect exact parameter selectors stored for the given C++ method. - void collectCXXMethodParameterSelectors( + bool collectCXXMethodParameterSelectors( ContextID CtxID, llvm::StringRef Name, llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors); @@ -202,7 +202,7 @@ class APINotesReader { std::optional<Context> Ctx = std::nullopt); /// Collect exact parameter selectors stored for the given global function. - void collectGlobalFunctionParameterSelectors( + bool collectGlobalFunctionParameterSelectors( llvm::StringRef Name, llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors, std::optional<Context> Ctx = std::nullopt); diff --git a/clang/include/clang/APINotes/Types.h b/clang/include/clang/APINotes/Types.h index db00c42f4561b..f0b6f589a7c5e 100644 --- a/clang/include/clang/APINotes/Types.h +++ b/clang/include/clang/APINotes/Types.h @@ -14,6 +14,7 @@ #include "llvm/ADT/StringRef.h" #include <climits> #include <optional> +#include <string> #include <vector> namespace llvm { @@ -22,6 +23,12 @@ class raw_ostream; namespace clang { namespace api_notes { + +std::string +formatAPINotesParameterSelector(llvm::ArrayRef<llvm::StringRef> Parameters); +std::string +formatAPINotesParameterSelector(llvm::ArrayRef<std::string> Parameters); + enum class RetainCountConventionKind { None, CFReturnsRetained, diff --git a/clang/lib/APINotes/APINotesReader.cpp b/clang/lib/APINotes/APINotesReader.cpp index 944beccb9d2df..9520fbac75b70 100644 --- a/clang/lib/APINotes/APINotesReader.cpp +++ b/clang/lib/APINotes/APINotesReader.cpp @@ -841,7 +841,7 @@ class APINotesReader::Implementation { /// Collect exact parameter selectors stored in the given function-like table. template <typename TableT> - void collectFunctionParameterSelectors( + bool collectFunctionParameterSelectors( TableT *Table, uint32_t ParentContextID, llvm::StringRef Name, llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors); @@ -923,36 +923,33 @@ APINotesReader::Implementation::getIdentifierString(IdentifierID ID) { } template <typename TableT> -void APINotesReader::Implementation::collectFunctionParameterSelectors( +bool APINotesReader::Implementation::collectFunctionParameterSelectors( TableT *Table, uint32_t ParentContextID, llvm::StringRef Name, llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors) { if (!Table) - return; + return true; std::optional<IdentifierID> NameID = getIdentifier(Name); if (!NameID) - return; + return true; - for (auto I = Table->key_begin(), E = Table->key_end(); I != E; ++I) { - FunctionTableKey Key = I.getInternalKey(); + for (FunctionTableKey Key : Table->keys()) { if (Key.parentContextID != ParentContextID || Key.nameID != static_cast<unsigned>(*NameID) || !Key.parameterTypeIDs) continue; llvm::SmallVector<std::string, 4> ParameterSelector; ParameterSelector.reserve(Key.parameterTypeIDs->size()); - bool Failed = false; for (IdentifierID TypeID : *Key.parameterTypeIDs) { std::optional<llvm::StringRef> TypeName = getIdentifierString(TypeID); - if (!TypeName) { - Failed = true; - break; - } + if (!TypeName) + return false; ParameterSelector.push_back(TypeName->str()); } - if (!Failed) - Selectors.push_back(std::move(ParameterSelector)); + Selectors.push_back(std::move(ParameterSelector)); } + + return true; } std::optional<FunctionTableKey> @@ -2413,10 +2410,10 @@ auto APINotesReader::lookupCXXMethod(ContextID CtxID, llvm::StringRef Name, return lookupCXXMethodImpl(CtxID, Name, Parameters); } -void APINotesReader::collectCXXMethodParameterSelectors( +bool APINotesReader::collectCXXMethodParameterSelectors( ContextID CtxID, llvm::StringRef Name, llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors) { - Implementation->collectFunctionParameterSelectors( + return Implementation->collectFunctionParameterSelectors( Implementation->CXXMethodTable.get(), CtxID.Value, Name, Selectors); } @@ -2487,12 +2484,12 @@ auto APINotesReader::lookupGlobalFunction( return lookupGlobalFunctionImpl(Name, Parameters, Ctx); } -void APINotesReader::collectGlobalFunctionParameterSelectors( +bool APINotesReader::collectGlobalFunctionParameterSelectors( llvm::StringRef Name, llvm::SmallVectorImpl<llvm::SmallVector<std::string, 4>> &Selectors, std::optional<Context> Ctx) { uint32_t ParentContextID = Ctx ? Ctx->id.Value : static_cast<uint32_t>(-1); - Implementation->collectFunctionParameterSelectors( + return Implementation->collectFunctionParameterSelectors( Implementation->GlobalFunctionTable.get(), ParentContextID, Name, Selectors); } diff --git a/clang/lib/APINotes/APINotesTypes.cpp b/clang/lib/APINotes/APINotesTypes.cpp index 96dd722587c10..7e3f71f719763 100644 --- a/clang/lib/APINotes/APINotesTypes.cpp +++ b/clang/lib/APINotes/APINotesTypes.cpp @@ -7,10 +7,35 @@ //===----------------------------------------------------------------------===// #include "clang/APINotes/Types.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Support/raw_ostream.h" +namespace { +template <typename ParameterT> +std::string +formatAPINotesParameterSelectorImpl(llvm::ArrayRef<ParameterT> Parameters) { + std::string Result; + llvm::raw_string_ostream OS(Result); + OS << "["; + llvm::interleaveComma(Parameters, OS); + OS << "]"; + return Result; +} +} // namespace + namespace clang { namespace api_notes { + +std::string +formatAPINotesParameterSelector(llvm::ArrayRef<llvm::StringRef> Parameters) { + return formatAPINotesParameterSelectorImpl(Parameters); +} + +std::string +formatAPINotesParameterSelector(llvm::ArrayRef<std::string> Parameters) { + return formatAPINotesParameterSelectorImpl(Parameters); +} + LLVM_DUMP_METHOD void CommonEntityInfo::dump(llvm::raw_ostream &OS) const { if (Unavailable) OS << "[Unavailable] (" << UnavailableMsg << ")" << ' '; diff --git a/clang/lib/APINotes/APINotesYAMLCompiler.cpp b/clang/lib/APINotes/APINotesYAMLCompiler.cpp index 058f9da338661..10db3fbb8e659 100644 --- a/clang/lib/APINotes/APINotesYAMLCompiler.cpp +++ b/clang/lib/APINotes/APINotesYAMLCompiler.cpp @@ -794,42 +794,17 @@ struct KnownFunctionSelector { llvm::ArrayRef<llvm::StringRef> Parameters; }; -static bool equalParameterSelectors(llvm::ArrayRef<llvm::StringRef> LHS, - llvm::ArrayRef<llvm::StringRef> RHS) { - if (LHS.size() != RHS.size()) - return false; - - for (unsigned I = 0, E = LHS.size(); I != E; ++I) - if (LHS[I] != RHS[I]) - return false; - - return true; -} - static bool hasFunctionSelector(llvm::ArrayRef<KnownFunctionSelector> KnownSelectors, llvm::StringRef Name, llvm::ArrayRef<llvm::StringRef> Parameters) { for (const KnownFunctionSelector &Known : KnownSelectors) - if (Known.Name == Name && - equalParameterSelectors(Known.Parameters, Parameters)) + if (Known.Name == Name && Known.Parameters == Parameters) return true; return false; } -static std::string -formatWhereParameters(llvm::ArrayRef<llvm::StringRef> Parameters) { - std::string Result = "["; - for (unsigned I = 0, E = Parameters.size(); I != E; ++I) { - if (I) - Result += ", "; - Result += Parameters[I]; - } - Result += "]"; - return Result; -} - class YAMLConverter { const Module &M; APINotesWriter Writer; @@ -1215,7 +1190,7 @@ class YAMLConverter { *WhereParameters.second)) { emitError(llvm::Twine("duplicate definition of C++ method '") + CXXMethod.Name + "' with Where.Parameters " + - formatWhereParameters(*WhereParameters.second)); + formatAPINotesParameterSelector(*WhereParameters.second)); continue; } KnownMethodSelectors.push_back( @@ -1309,7 +1284,7 @@ class YAMLConverter { *WhereParameters.second)) { emitError(llvm::Twine("duplicate definition of global function '") + Function.Name + "' with Where.Parameters " + - formatWhereParameters(*WhereParameters.second)); + formatAPINotesParameterSelector(*WhereParameters.second)); continue; } KnownFunctionSelectors.push_back( diff --git a/clang/lib/Sema/SemaAPINotes.cpp b/clang/lib/Sema/SemaAPINotes.cpp index e9e90c93755d3..043af7e57165b 100644 --- a/clang/lib/Sema/SemaAPINotes.cpp +++ b/clang/lib/Sema/SemaAPINotes.cpp @@ -1124,20 +1124,7 @@ makeParameterSelectorSet(ArrayRef<SmallVector<std::string, 4>> Selectors) { return Set; } -static std::string -formatParameterSelectorForDiagnostic(ArrayRef<std::string> Parameters) { - std::string Result = "["; - for (unsigned I = 0, E = Parameters.size(); I != E; ++I) { - if (I) - Result += ", "; - Result += Parameters[I]; - } - Result += "]"; - return Result; -} - -static void collectOverloadParameterSelectors(const Sema &S, - const FunctionDecl *FD, +static void collectOverloadParameterSelectors(const Sema &S, FunctionDecl *FD, APINotesParameterSelectorSet &Set, bool &IsRepresentative) { IsRepresentative = false; @@ -1153,16 +1140,8 @@ static void collectOverloadParameterSelectors(const Sema &S, if (auto Candidates = getAPINotesParameterSelectorCandidates(S, Candidate)) Set.add(*Candidates); }; - - for (NamedDecl *ND : FD->getDeclContext()->lookup(FD->getDeclName())) { - if (auto *Candidate = dyn_cast<FunctionDecl>(ND)) - AddCandidate(Candidate); - } - - if (FoundRepresentative) - return; - - for (Decl *D : FD->getDeclContext()->decls()) { + + for (Decl *D : FD->getDeclContext()->noload_decls()) { auto *ND = dyn_cast<NamedDecl>(D); if (!ND || ND->getDeclName() != FD->getDeclName()) continue; @@ -1189,7 +1168,8 @@ static void diagnoseUnmatchedParameterSelectors( S.Diag(Loc, diag::warn_apinotes_message) << (llvm::Twine("API notes entry for '") + Name + "' has unmatched Where.Parameters " + - formatParameterSelectorForDiagnostic(APINotesSelector.Parameters)) + api_notes::formatAPINotesParameterSelector( + APINotesSelector.Parameters)) .str(); } } @@ -1274,13 +1254,14 @@ void Sema::ProcessAPINotes(Decl *D) { if (DiagnoseUnmatchedSelectors && !DeclarationSelectors.empty()) { SmallVector<SmallVector<std::string, 4>, 4> RawAPINotesSelectors; - Reader->collectGlobalFunctionParameterSelectors( - FD->getName(), RawAPINotesSelectors, APINotesContext); - APINotesParameterSelectorSet APINotesSelectors = - makeParameterSelectorSet(RawAPINotesSelectors); - diagnoseUnmatchedParameterSelectors( - *this, FD->getLocation(), FD->getName(), APINotesSelectors, - DeclarationSelectors); + if (Reader->collectGlobalFunctionParameterSelectors( + FD->getName(), RawAPINotesSelectors, APINotesContext)) { + APINotesParameterSelectorSet APINotesSelectors = + makeParameterSelectorSet(RawAPINotesSelectors); + diagnoseUnmatchedParameterSelectors( + *this, FD->getLocation(), FD->getName(), APINotesSelectors, + DeclarationSelectors); + } } } } @@ -1495,13 +1476,14 @@ void Sema::ProcessAPINotes(Decl *D) { DiagnoseUnmatchedSelectors); if (DiagnoseUnmatchedSelectors && !DeclarationSelectors.empty()) { SmallVector<SmallVector<std::string, 4>, 4> RawAPINotesSelectors; - Reader->collectCXXMethodParameterSelectors( - Context->id, MethodName, RawAPINotesSelectors); - APINotesParameterSelectorSet APINotesSelectors = - makeParameterSelectorSet(RawAPINotesSelectors); - diagnoseUnmatchedParameterSelectors( - *this, CXXMethod->getLocation(), MethodName, - APINotesSelectors, DeclarationSelectors); + if (Reader->collectCXXMethodParameterSelectors( + Context->id, MethodName, RawAPINotesSelectors)) { + APINotesParameterSelectorSet APINotesSelectors = + makeParameterSelectorSet(RawAPINotesSelectors); + diagnoseUnmatchedParameterSelectors( + *this, CXXMethod->getLocation(), MethodName, + APINotesSelectors, DeclarationSelectors); + } } } } >From 1017bcf15acf65712f365a5c2ffdeef8a2c37765 Mon Sep 17 00:00:00 2001 From: stoeckoverflow <[email protected]> Date: Tue, 14 Jul 2026 14:12:05 +0200 Subject: [PATCH 3/3] [APINotes] Fix formatting in Where.Parameters diagnostics --- clang/lib/Sema/SemaAPINotes.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaAPINotes.cpp b/clang/lib/Sema/SemaAPINotes.cpp index 043af7e57165b..c2ec720b7120e 100644 --- a/clang/lib/Sema/SemaAPINotes.cpp +++ b/clang/lib/Sema/SemaAPINotes.cpp @@ -1140,7 +1140,7 @@ static void collectOverloadParameterSelectors(const Sema &S, FunctionDecl *FD, if (auto Candidates = getAPINotesParameterSelectorCandidates(S, Candidate)) Set.add(*Candidates); }; - + for (Decl *D : FD->getDeclContext()->noload_decls()) { auto *ND = dyn_cast<NamedDecl>(D); if (!ND || ND->getDeclName() != FD->getDeclName()) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
