llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Paul Kirth (ilovepi) <details> <summary>Changes</summary> This adds a few FixIts that update the usage of namespaces other than LIBC_NAMESPACE_DECL, and add the required header when its missing. --- Full diff: https://github.com/llvm/llvm-project/pull/99681.diff 4 Files Affected: - (modified) clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp (+33-4) - (modified) clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h (+11-1) - (modified) clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h (+2) - (modified) clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp (+5-1) ``````````diff diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp index bb436e4d12a30..e6e4900283c79 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp @@ -8,7 +8,6 @@ #include "ImplementationInNamespaceCheck.h" #include "NamespaceConstants.h" -#include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" using namespace clang::ast_matchers; @@ -25,6 +24,16 @@ void ImplementationInNamespaceCheck::registerMatchers(MatchFinder *Finder) { this); } +void ImplementationInNamespaceCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle()); +} + +void ImplementationInNamespaceCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + IncludeInserter.registerPreprocessor(PP); +} + void ImplementationInNamespaceCheck::check( const MatchFinder::MatchResult &Result) { const auto *MatchedDecl = @@ -41,8 +50,26 @@ void ImplementationInNamespaceCheck::check( // Enforce that the namespace is the result of macro expansion if (Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) == false) { - diag(NS->getLocation(), "the outermost namespace should be the '%0' macro") - << RequiredNamespaceDeclMacroName; + auto DB = diag(NS->getLocation(), + "the outermost namespace should be the '%0' macro") + << RequiredNamespaceDeclMacroName; + + // TODO: Determine how to split inline namespaces correctly in the FixItHint + // + // We can't easily replace LIBC_NAMEPACE::inner::namespace { with + // + // namespace LIBC_NAMEPACE_DECL { + // namespace inner::namespace { + // + // For now, just update the simple case w/ LIBC_NAMEPACE_DECL + if (!NS->isInlineNamespace()) + DB << FixItHint::CreateReplacement(NS->getLocation(), + RequiredNamespaceDeclMacroName); + + DB << IncludeInserter.createIncludeInsertion( + Result.SourceManager->getFileID(NS->getBeginLoc()), + NamespaceMacroHeader); + return; } @@ -51,7 +78,9 @@ void ImplementationInNamespaceCheck::check( // instead. if (NS->getVisibility() != Visibility::HiddenVisibility) { diag(NS->getLocation(), "the '%0' macro should start with '%1'") - << RequiredNamespaceDeclMacroName << RequiredNamespaceDeclStart; + << RequiredNamespaceDeclMacroName << RequiredNamespaceDeclStart + << FixItHint::CreateReplacement(NS->getLocation(), + RequiredNamespaceDeclMacroName); return; } diff --git a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h index 42da38f728bb8..f7fc19fda180a 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h +++ b/clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_IMPLEMENTATIONINNAMESPACECHECK_H #include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" namespace clang::tidy::llvm_libc { @@ -20,13 +21,22 @@ namespace clang::tidy::llvm_libc { class ImplementationInNamespaceCheck : public ClangTidyCheck { public: ImplementationInNamespaceCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context), + IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + utils::IncludeInserter IncludeInserter; }; } // namespace clang::tidy::llvm_libc diff --git a/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h index 83908a7875d03..db85c7b8b93f0 100644 --- a/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h +++ b/clang-tools-extra/clang-tidy/llvmlibc/NamespaceConstants.h @@ -16,5 +16,7 @@ const static llvm::StringRef RequiredNamespaceDeclStart = "[[gnu::visibility(\"hidden\")]] __llvm_libc"; const static llvm::StringRef RequiredNamespaceDeclMacroName = "LIBC_NAMESPACE_DECL"; +const static llvm::StringRef + NamespaceMacroHeader("src/__support/macros/config.h"); } // namespace clang::tidy::llvm_libc diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp index 4a5576f2c5d62..ad60c04265533 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvmlibc/implementation-in-namespace.cpp @@ -1,4 +1,6 @@ -// RUN: %check_clang_tidy %s llvmlibc-implementation-in-namespace %t +// RUN: %check_clang_tidy %s llvmlibc-implementation-in-namespace %t -fix + +// CHECK-FIXES: #include "src/__support/macros/config.h" #define MACRO_A "defining macros outside namespace is valid" @@ -15,6 +17,7 @@ void funcF() {} namespace outer_most { // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE_DECL' macro +// CHECK-FIXES: namespace LIBC_NAMESPACE_DECL { class A {}; } @@ -26,6 +29,7 @@ namespace { namespace namespaceG { // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the outermost namespace should be the 'LIBC_NAMESPACE_DECL' macro +// CHECK-FIXES: namespace LIBC_NAMESPACE_DECL { namespace __llvm_libc { namespace namespaceH { class ClassB; `````````` </details> https://github.com/llvm/llvm-project/pull/99681 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
