llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: StoeckOverflow <details> <summary>Changes</summary> This PR normalizes exact `Where.Parameters` selector spellings and adds focused unit coverage for declaration-side selector extraction. It teaches API notes conversion to normalize selector strings before storing, comparing, and diagnosing them, so supported equivalent spellings such as `int *` and `int*`, `Box<int, double>` and `Box<int,double>`, or top-level by-value / pointer `const` variants compare consistently with Sema-created declaration selectors. The normalizer is intentionally lexical. It does not parse selector strings as Clang types and does not broaden `Where.Parameters` into canonical type matching. It moves the declaration-side selector code from `SemaAPINotes.cpp` into `clang/Sema/APINotesSelector.h` and `clang/lib/Sema/APINotesSelector.cpp`. The new helper, `getAPINotesParameterSelectorCandidates`, builds the source-spelling selector and optional desugared fallback selector for a `FunctionDecl`. `SemaAPINotes.cpp` now calls that helper for exact `Where.Parameters` matching and unmatched-selector diagnostics, while keeping API notes lookup, note application, and diagnostic emission there. The new `APINotesSelectorTest` unit test builds small Clang ASTs from declaration snippets and checks the selector strings returned by `getAPINotesParameterSelectorCandidates`. Tests cover whitespace normalization, pointer/reference punctuation spacing, template punctuation spacing, supported top-level `const` normalization, normalized duplicate-selector diagnostics, zero-parameter selectors, multi-parameter/defaulted declarations, alias and deep-alias fallback, and Objective-C++ nullability stripping. Existing Sema/API-notes coverage continues to exercise global-function and C++-method matching paths. Reviewers: @<!-- -->Xazax-hun @<!-- -->j-hui @<!-- -->egorzhdan --- Patch is 45.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/213043.diff 14 Files Affected: - (modified) clang/include/clang/APINotes/Types.h (+3) - (added) clang/include/clang/Sema/APINotesSelector.h (+44) - (modified) clang/lib/APINotes/APINotesTypes.cpp (+117) - (modified) clang/lib/APINotes/APINotesYAMLCompiler.cpp (+57-16) - (added) clang/lib/Sema/APINotesSelector.cpp (+85) - (modified) clang/lib/Sema/CMakeLists.txt (+1) - (modified) clang/lib/Sema/SemaAPINotes.cpp (+3-83) - (added) clang/test/APINotes/Inputs/Headers/WhereParametersNormalization.apinotes (+131) - (added) clang/test/APINotes/Inputs/Headers/WhereParametersNormalization.h (+49) - (modified) clang/test/APINotes/Inputs/Headers/module.modulemap (+5) - (modified) clang/test/APINotes/where-parameters-diagnostics.cpp (+24) - (added) clang/test/APINotes/where-parameters-normalization.cpp (+112) - (added) clang/unittests/Sema/APINotesSelectorTest.cpp (+166) - (modified) clang/unittests/Sema/CMakeLists.txt (+1) ``````````diff diff --git a/clang/include/clang/APINotes/Types.h b/clang/include/clang/APINotes/Types.h index af989d3a1b7f0..a839c08300b40 100644 --- a/clang/include/clang/APINotes/Types.h +++ b/clang/include/clang/APINotes/Types.h @@ -66,6 +66,9 @@ enum class SwiftNewTypeKind { enum class SwiftSafetyKind { Unspecified, Safe, Unsafe, None }; +/// Normalize an API notes parameter selector spelling for matching. +std::string normalizeAPINotesParameterSelector(llvm::StringRef Spelling); + /// Describes API notes data for any entity. /// /// This is used as the base of all API notes. diff --git a/clang/include/clang/Sema/APINotesSelector.h b/clang/include/clang/Sema/APINotesSelector.h new file mode 100644 index 0000000000000..8a95437354a4e --- /dev/null +++ b/clang/include/clang/Sema/APINotesSelector.h @@ -0,0 +1,44 @@ +//===--- APINotesSelector.h - API Notes selector helpers --------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_SEMA_APINOTESSELECTOR_H +#define LLVM_CLANG_SEMA_APINOTESSELECTOR_H + +#include "llvm/ADT/SmallVector.h" +#include <optional> +#include <string> + +namespace clang { + +class ASTContext; +class FunctionDecl; + +struct APINotesParameterSelector { + llvm::SmallVector<std::string, 4> Parameters; + + bool operator==(const APINotesParameterSelector &Other) const { + return Parameters == Other.Parameters; + } + + bool operator!=(const APINotesParameterSelector &Other) const { + return !(*this == Other); + } +}; + +struct APINotesParameterSelectorCandidates { + APINotesParameterSelector Source; + std::optional<APINotesParameterSelector> Desugared; +}; + +std::optional<APINotesParameterSelectorCandidates> +getAPINotesParameterSelectorCandidates(const ASTContext &Context, + const FunctionDecl *FD); + +} // namespace clang + +#endif // LLVM_CLANG_SEMA_APINOTESSELECTOR_H diff --git a/clang/lib/APINotes/APINotesTypes.cpp b/clang/lib/APINotes/APINotesTypes.cpp index c8b9272aa0ab7..35ea18a0ebfda 100644 --- a/clang/lib/APINotes/APINotesTypes.cpp +++ b/clang/lib/APINotes/APINotesTypes.cpp @@ -7,11 +7,128 @@ //===----------------------------------------------------------------------===// #include "clang/APINotes/Types.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Support/raw_ostream.h" +#include <tuple> namespace clang { namespace api_notes { +// Conservatively detect spellings where cv-qualification belongs to an +// indirect/declarator layer rather than the by-value parameter itself. +static bool +hasTopLevelIndirectParameterSelectorSpelling(llvm::StringRef Spelling) { + unsigned Depth = 0; + + for (char C : Spelling) { + switch (C) { + case '*': + case '&': + if (Depth == 0) + return true; + break; + + case '[': + case '(': + if (Depth == 0) + return true; + ++Depth; + break; + + case '<': + ++Depth; + break; + + case '>': + case ']': + case ')': + if (Depth != 0) + --Depth; + break; + + default: + break; + } + } + + return false; +} + +static bool shouldDropParameterSelectorSpace(char Previous, char Next) { + if (Previous == '<' || Previous == ',' || Next == '>' || Next == ',' || + Next == '<') + return true; + + if (Next == '*' || Next == '&') + return true; + + if (Previous == '&' && Next == '&') + return true; + + return false; +} + +static void +collapseParameterSelectorWhitespace(llvm::StringRef Spelling, + llvm::SmallVectorImpl<char> &Collapsed) { + Collapsed.clear(); + while (!Spelling.empty()) { + llvm::StringRef Token; + std::tie(Token, Spelling) = llvm::getToken(Spelling); + if (Token.empty()) + break; + + if (!Collapsed.empty()) + Collapsed.push_back(' '); + Collapsed.append(Token.begin(), Token.end()); + } +} + +static llvm::StringRef stripTopLevelValueConst(llvm::StringRef Spelling) { + if (!hasTopLevelIndirectParameterSelectorSpelling(Spelling)) + Spelling.consume_front("const "); + Spelling.consume_back(" const"); + return Spelling; +} + +// Remove spaces around selector punctuation while preserving token-separating +// spaces such as the one in "unsigned int". +static void removeParameterSelectorPunctuationSpaces( + llvm::StringRef Spelling, llvm::SmallVectorImpl<char> &Normalized) { + Normalized.clear(); + for (unsigned I = 0, E = Spelling.size(); I != E; ++I) { + char C = Spelling[I]; + if (C == ' ' && I != 0 && I + 1 != E && + shouldDropParameterSelectorSpace(Spelling[I - 1], Spelling[I + 1])) + continue; + + Normalized.push_back(C); + } +} + +static std::string stripTopLevelPointerConst(llvm::StringRef Spelling) { + if (!Spelling.consume_back("*const")) + return Spelling.str(); + + std::string WithoutTopLevelConst = Spelling.str(); + WithoutTopLevelConst += '*'; + return WithoutTopLevelConst; +} + +std::string normalizeAPINotesParameterSelector(llvm::StringRef Spelling) { + llvm::SmallString<32> Collapsed; + collapseParameterSelectorWhitespace(Spelling, Collapsed); + + llvm::StringRef WithoutTopLevelValueConst = + stripTopLevelValueConst(Collapsed); + + llvm::SmallString<32> WithoutPunctuationSpaces; + removeParameterSelectorPunctuationSpaces(WithoutTopLevelValueConst, + WithoutPunctuationSpaces); + return stripTopLevelPointerConst(WithoutPunctuationSpaces); +} + 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 4079675228a21..e9b6cc847952c 100644 --- a/clang/lib/APINotes/APINotesYAMLCompiler.cpp +++ b/clang/lib/APINotes/APINotesYAMLCompiler.cpp @@ -21,6 +21,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/VersionTuple.h" @@ -809,6 +810,31 @@ getFunctionSelectorKey(llvm::StringRef Name, return Key.str().str(); } +// YAML conversion has parameter spellings but no AST context. Keep this as a +// narrow lexical normalization step. Declaration spellings are normalized with +// QualType in Sema before using the same lexical selector normalization. +static void normalizeWhereParameterList( + llvm::ArrayRef<llvm::StringRef> Parameters, + llvm::SmallVectorImpl<std::string> &NormalizedParameters) { + NormalizedParameters.clear(); + NormalizedParameters.reserve(Parameters.size()); + + for (llvm::StringRef Parameter : Parameters) + NormalizedParameters.push_back( + normalizeAPINotesParameterSelector(Parameter)); +} + +static llvm::SmallVector<llvm::StringRef, 4> +getParameterSelectorRefs(llvm::ArrayRef<std::string> Parameters) { + llvm::SmallVector<llvm::StringRef, 4> ParameterRefs; + ParameterRefs.reserve(Parameters.size()); + + for (const std::string &Parameter : Parameters) + ParameterRefs.push_back(Parameter); + + return ParameterRefs; +} + class YAMLConverter { const Module &M; APINotesWriter Writer; @@ -1190,24 +1216,32 @@ class YAMLConverter { continue; if (WhereParameters.second) { + llvm::SmallVector<std::string, 4> NormalizedWhereParameters; + normalizeWhereParameterList(*WhereParameters.second, + NormalizedWhereParameters); + auto NormalizedWhereParameterRefs = + getParameterSelectorRefs(NormalizedWhereParameters); if (!KnownMethodSelectors .insert(getFunctionSelectorKey(CXXMethod.Name, - *WhereParameters.second)) + NormalizedWhereParameterRefs)) .second) { emitError(llvm::Twine("multiple API notes entries for C++ method '") + CXXMethod.Name + "' with Where.Parameters " + - formatAPINotesParameterSelector(*WhereParameters.second)); + api_notes::formatAPINotesParameterSelector( + NormalizedWhereParameters)); continue; } + + CXXMethodInfo MI; + convertFunction(CXXMethod, MI); + Writer.addCXXMethod(TagCtxID, CXXMethod.Name, + NormalizedWhereParameterRefs, MI, SwiftVersion); + continue; } CXXMethodInfo MI; convertFunction(CXXMethod, MI); - if (WhereParameters.second) - Writer.addCXXMethod(TagCtxID, CXXMethod.Name, *WhereParameters.second, - MI, SwiftVersion); - else - Writer.addCXXMethod(TagCtxID, CXXMethod.Name, MI, SwiftVersion); + Writer.addCXXMethod(TagCtxID, CXXMethod.Name, MI, SwiftVersion); } // Convert nested tags. @@ -1284,21 +1318,32 @@ class YAMLConverter { continue; if (WhereParameters.second) { + llvm::SmallVector<std::string, 4> NormalizedWhereParameters; + normalizeWhereParameterList(*WhereParameters.second, + NormalizedWhereParameters); + auto NormalizedWhereParameterRefs = + getParameterSelectorRefs(NormalizedWhereParameters); if (!KnownFunctionSelectors .insert(getFunctionSelectorKey(Function.Name, - *WhereParameters.second)) + NormalizedWhereParameterRefs)) .second) { emitError( llvm::Twine("multiple API notes entries for global function '") + Function.Name + "' with Where.Parameters " + - formatAPINotesParameterSelector(*WhereParameters.second)); + formatAPINotesParameterSelector(NormalizedWhereParameters)); continue; } + + GlobalFunctionInfo GFI; + convertFunction(Function, GFI); + Writer.addGlobalFunction(Ctx, Function.Name, + NormalizedWhereParameterRefs, GFI, + SwiftVersion); + continue; } // Check for duplicate name-only global functions. - if (!WhereParameters.second && - !KnownNameOnlyFunctions.insert(Function.Name).second) { + if (!KnownNameOnlyFunctions.insert(Function.Name).second) { emitError(llvm::Twine("multiple definitions of global function '") + Function.Name + "'"); continue; @@ -1306,11 +1351,7 @@ class YAMLConverter { GlobalFunctionInfo GFI; convertFunction(Function, GFI); - if (WhereParameters.second) - Writer.addGlobalFunction(Ctx, Function.Name, *WhereParameters.second, - GFI, SwiftVersion); - else - Writer.addGlobalFunction(Ctx, Function.Name, GFI, SwiftVersion); + Writer.addGlobalFunction(Ctx, Function.Name, GFI, SwiftVersion); } // Write all enumerators. diff --git a/clang/lib/Sema/APINotesSelector.cpp b/clang/lib/Sema/APINotesSelector.cpp new file mode 100644 index 0000000000000..c45c0f7d28927 --- /dev/null +++ b/clang/lib/Sema/APINotesSelector.cpp @@ -0,0 +1,85 @@ +//===--- APINotesSelector.cpp - API Notes selector helpers ----------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Sema/APINotesSelector.h" +#include "clang/APINotes/Types.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/PrettyPrinter.h" +#include "clang/AST/Type.h" + +using namespace clang; + +namespace { + +void stripAPINotesParameterNullability(QualType &ParamType) { + while (true) { + if (!AttributedType::stripOuterNullability(ParamType)) + return; + } +} + +PrintingPolicy +getAPINotesParameterSelectorPrintingPolicy(const ASTContext &Context) { + PrintingPolicy Policy(Context.getLangOpts()); + Policy.PrintAsCanonical = false; + Policy.FullyQualifiedName = false; + Policy.SuppressScope = false; + Policy.UsePreferredNames = false; + Policy.MSVCFormatting = false; + Policy.SplitTemplateClosers = false; + Policy.IncludeNewlines = false; + return Policy; +} + +// Print the APINotes selector spelling for one parameter. The source-spelled +// selector is tried first. The desugared spelling is only a permissive +// fallback. +std::string getAPINotesParameterSelectorSpelling(QualType ParamType, + const ASTContext &Context, + const PrintingPolicy &Policy, + bool Desugar) { + if (Desugar) + ParamType = ParamType.getDesugaredType(Context); + + ParamType.removeLocalConst(); + stripAPINotesParameterNullability(ParamType); + + return api_notes::normalizeAPINotesParameterSelector( + ParamType.getAsString(Policy)); +} + +} // namespace + +std::optional<APINotesParameterSelectorCandidates> +clang::getAPINotesParameterSelectorCandidates(const ASTContext &Context, + const FunctionDecl *FD) { + const auto *FPT = FD->getType()->getAs<FunctionProtoType>(); + if (!FPT) + return std::nullopt; + + APINotesParameterSelectorCandidates Candidates; + APINotesParameterSelector Desugared; + Candidates.Source.Parameters.reserve(FPT->getNumParams()); + Desugared.Parameters.reserve(FPT->getNumParams()); + + const PrintingPolicy Policy = + getAPINotesParameterSelectorPrintingPolicy(Context); + for (QualType ParamType : FPT->param_types()) { + Candidates.Source.Parameters.push_back( + getAPINotesParameterSelectorSpelling(ParamType, Context, Policy, + /*Desugar=*/false)); + Desugared.Parameters.push_back(getAPINotesParameterSelectorSpelling( + ParamType, Context, Policy, /*Desugar=*/true)); + } + + if (Candidates.Source != Desugared) + Candidates.Desugared = std::move(Desugared); + + return Candidates; +} diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt index 88f0c993888d9..7ab1916eb568b 100644 --- a/clang/lib/Sema/CMakeLists.txt +++ b/clang/lib/Sema/CMakeLists.txt @@ -14,6 +14,7 @@ clang_tablegen(OpenCLBuiltins.inc -gen-clang-opencl-builtins ) add_clang_library(clangSema + APINotesSelector.cpp AnalysisBasedWarnings.cpp CheckExprLifetime.cpp CodeCompleteConsumer.cpp diff --git a/clang/lib/Sema/SemaAPINotes.cpp b/clang/lib/Sema/SemaAPINotes.cpp index c5560605124c8..0813e5a6603b2 100644 --- a/clang/lib/Sema/SemaAPINotes.cpp +++ b/clang/lib/Sema/SemaAPINotes.cpp @@ -21,6 +21,7 @@ #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" #include "clang/Basic/SourceLocation.h" #include "clang/Lex/Lexer.h" +#include "clang/Sema/APINotesSelector.h" #include "clang/Sema/SemaObjC.h" #include "clang/Sema/SemaSwift.h" #include <stack> @@ -994,87 +995,6 @@ UnwindTagContext(TagDecl *DC, api_notes::APINotesManager &APINotes) { return std::nullopt; } -static void stripAPINotesParameterNullability(QualType &ParamType) { - while (true) { - if (!AttributedType::stripOuterNullability(ParamType)) - return; - } -} - -namespace clang { -struct APINotesParameterSelector { - SmallVector<std::string, 4> Parameters; - - bool operator==(const APINotesParameterSelector &Other) const { - return Parameters == Other.Parameters; - } - - bool operator!=(const APINotesParameterSelector &Other) const { - return !(*this == Other); - } -}; - -struct APINotesParameterSelectorCandidates { - APINotesParameterSelector Source; - std::optional<APINotesParameterSelector> Desugared; -}; -} // namespace clang - -static PrintingPolicy -getAPINotesParameterSelectorPrintingPolicy(const ASTContext &Context) { - PrintingPolicy Policy(Context.getLangOpts()); - Policy.PrintAsCanonical = false; - Policy.FullyQualifiedName = false; - Policy.SuppressScope = false; - Policy.UsePreferredNames = false; - Policy.MSVCFormatting = false; - Policy.SplitTemplateClosers = false; - Policy.IncludeNewlines = false; - return Policy; -} - -// Print the APINotes selector spelling for one parameter. The source-spelled -// selector is tried first. The desugared spelling is only a permissive -// fallback. -static std::string getAPINotesParameterSelectorSpelling( - QualType ParamType, const ASTContext &Context, const PrintingPolicy &Policy, - bool Desugar) { - if (Desugar) - ParamType = ParamType.getDesugaredType(Context); - - ParamType.removeLocalConst(); - stripAPINotesParameterNullability(ParamType); - - return ParamType.getAsString(Policy); -} - -static std::optional<APINotesParameterSelectorCandidates> -getAPINotesParameterSelectorCandidates(const Sema &S, const FunctionDecl *FD) { - const auto *FPT = FD->getType()->getAs<FunctionProtoType>(); - if (!FPT) - return std::nullopt; - - APINotesParameterSelectorCandidates Candidates; - APINotesParameterSelector Desugared; - Candidates.Source.Parameters.reserve(FPT->getNumParams()); - Desugared.Parameters.reserve(FPT->getNumParams()); - - const PrintingPolicy Policy = - getAPINotesParameterSelectorPrintingPolicy(S.Context); - for (QualType ParamType : FPT->param_types()) { - Candidates.Source.Parameters.push_back( - getAPINotesParameterSelectorSpelling(ParamType, S.Context, Policy, - /*Desugar=*/false)); - Desugared.Parameters.push_back(getAPINotesParameterSelectorSpelling( - ParamType, S.Context, Policy, /*Desugar=*/true)); - } - - if (Candidates.Source != Desugared) - Candidates.Desugared = std::move(Desugared); - - return Candidates; -} - APINotesSelectorDiagnosticReaderState & APINotesSelectorDiagnosticState::getOrCreateReaderState( api_notes::APINotesReader &Reader) { @@ -1169,7 +1089,7 @@ void Sema::ProcessAPINotes(Decl *D) { if (auto FD = dyn_cast<FunctionDecl>(D)) { if (FD->getDeclName().isIdentifier()) { auto ParameterSelectorCandidates = - getAPINotesParameterSelectorCandidates(*this, FD); + getAPINotesParameterSelectorCandidates(Context, FD); for (auto Reader : Readers) { auto Info = @@ -1382,7 +1302,7 @@ void Sema::ProcessAPINotes(Decl *D) { !isa<CXXDestructorDecl>(CXXMethod) && !isa<CXXConversionDecl>(CXXMethod)) { auto ParameterSelectorCandidates = - getAPINotesParameterSelectorCandidates(*this, CXXMethod); + getAPINotesParameterSelectorCandidates(getASTContext(), CXXMethod); for (auto Reader : Readers) { if (auto Context = UnwindTagContext(TagContext, APINotes)) { std::string MethodName; diff --git a/clang/test/APINotes/Inputs/Headers/WhereParametersNormalization.apinotes b/clang/test/APINotes/Inputs/Headers/WhereParametersNormalization.apinotes new file mode 100644 index 0000000000000..6c9ed80e4a431 --- /dev/null +++ b/clang/test/APINotes/Inputs/Headers/WhereParametersNormalization.apinotes @@ -0,0 +1,131 @@ +--- +Name: WhereParametersNormalization +Functions: +- Name: normalizedEmpty + Where: + Parameters: [] + SwiftName: normalizedEmpty() +- Name: normalizedDefaults + Where: + Parameters: + - int + - double + SwiftName: normalizedDefaults(_:_:) +- Name: normalizedWhitespace + Where: + Parameters: + - ' unsigned int ' + SwiftName: normalizedWhitespace(_:) +- Name: normalizedTemplateSpacing + Where: + Parameters: + - 'NormalizationBox<int,double>' + SwiftName: normalizedTemplateSpacing(_:) +- Name: normalizedPointerSpacing + Where: + Parameters: + - 'int*' + SwiftName: normalizedPointerSpacing(_:) +- Name: normalizedRValueReferenceSpacing + Where: + Parameters: + - ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/213043 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
