Hi alexfh, klimek, djasper,
The implementation is split into a generic part and a LLVM-specific part.
Other codebases can implement it with their own style. The specific features
supported are:
- Verification (and fixing) of header guards against a style based on the file
path
- Automatic insertion of header guards for headers that are missing them
- A warning when the header guard doesn't enable our fancy header guard
optimization
(e.g. when there's an include preceeding the guard)
- Automatic insertion of a comment with the guard name after #endif.
For the LLVM style we disable #endif comments for now, they're not very common
in the codebase. We also only flag headers in the include directories, there
doesn't seem to be a common style outside.
http://reviews.llvm.org/D4867
Files:
clang-tidy/CMakeLists.txt
clang-tidy/llvm/CMakeLists.txt
clang-tidy/llvm/HeaderGuardCheck.cpp
clang-tidy/llvm/HeaderGuardCheck.h
clang-tidy/llvm/LLVMTidyModule.cpp
clang-tidy/utils/CMakeLists.txt
clang-tidy/utils/HeaderGuard.cpp
clang-tidy/utils/HeaderGuard.h
unittests/clang-tidy/LLVMModuleTest.cpp
Index: clang-tidy/CMakeLists.txt
===================================================================
--- clang-tidy/CMakeLists.txt
+++ clang-tidy/CMakeLists.txt
@@ -27,3 +27,4 @@
add_subdirectory(llvm)
add_subdirectory(google)
add_subdirectory(misc)
+add_subdirectory(utils)
Index: clang-tidy/llvm/CMakeLists.txt
===================================================================
--- clang-tidy/llvm/CMakeLists.txt
+++ clang-tidy/llvm/CMakeLists.txt
@@ -1,6 +1,7 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyLLVMModule
+ HeaderGuardCheck.cpp
IncludeOrderCheck.cpp
LLVMTidyModule.cpp
NamespaceCommentCheck.cpp
@@ -12,4 +13,5 @@
clangBasic
clangLex
clangTidy
+ clangTidyUtils
)
Index: clang-tidy/llvm/HeaderGuardCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/llvm/HeaderGuardCheck.cpp
@@ -0,0 +1,36 @@
+//===--- HeaderGuardCheck.cpp - clang-tidy --------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "HeaderGuardCheck.h"
+
+namespace clang {
+namespace tidy {
+
+bool LLVMHeaderGuardCheck::shouldFixHeaderGuard(StringRef Filename) {
+ return Filename.find("include") != StringRef::npos;
+}
+
+std::string LLVMHeaderGuardCheck::getHeaderGuard(StringRef Filename,
+ StringRef OldGuard) {
+ size_t PostInclude = Filename.find("include/") + std::strlen("include/");
+ std::string Guard = Filename.substr(PostInclude);
+
+ std::replace(Guard.begin(), Guard.end(), '/', '_');
+ std::replace(Guard.begin(), Guard.end(), '.', '_');
+ std::replace(Guard.begin(), Guard.end(), '-', '_');
+
+ // The prevalent style in clang is LLVM_CLANG_FOO_BAR_H
+ if (StringRef(Guard).startswith("clang"))
+ Guard = "LLVM_" + Guard;
+
+ return StringRef(Guard).upper();
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/llvm/HeaderGuardCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/llvm/HeaderGuardCheck.h
@@ -0,0 +1,30 @@
+//===--- HeaderGuardCheck.h - clang-tidy ------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_HEADER_GUARD_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_HEADER_GUARD_CHECK_H
+
+#include "../utils/HeaderGuard.h"
+
+namespace clang {
+namespace tidy {
+
+/// Finds and fixes header guards that do not adhere to LLVM style.
+class LLVMHeaderGuardCheck : public HeaderGuardCheck {
+ public:
+ bool shouldSuggestEndifComment(StringRef Filename) { return false; }
+ bool shouldFixHeaderGuard(StringRef Filename) override;
+ std::string getHeaderGuard(StringRef Filename,
+ StringRef OldGuard = StringRef()) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_HEADER_GUARD_CHECK_H
Index: clang-tidy/llvm/LLVMTidyModule.cpp
===================================================================
--- clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tidy/llvm/LLVMTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "HeaderGuardCheck.h"
#include "IncludeOrderCheck.h"
#include "NamespaceCommentCheck.h"
#include "TwineLocalCheck.h"
@@ -21,6 +22,8 @@
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.addCheckFactory(
+ "llvm-header-guard", new ClangTidyCheckFactory<LLVMHeaderGuardCheck>());
+ CheckFactories.addCheckFactory(
"llvm-include-order", new ClangTidyCheckFactory<IncludeOrderCheck>());
CheckFactories.addCheckFactory(
"llvm-namespace-comment",
Index: clang-tidy/utils/CMakeLists.txt
===================================================================
--- /dev/null
+++ clang-tidy/utils/CMakeLists.txt
@@ -0,0 +1,12 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyUtils
+ HeaderGuard.cpp
+
+ LINK_LIBS
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTidy
+ )
Index: clang-tidy/utils/HeaderGuard.cpp
===================================================================
--- /dev/null
+++ clang-tidy/utils/HeaderGuard.cpp
@@ -0,0 +1,220 @@
+//===--- HeaderGuard.cpp - clang-tidy -------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "HeaderGuard.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Path.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief canonicalize a path by removing ./ and ../ components.
+static std::string cleanPath(StringRef Path) {
+ SmallString<256> NewPath;
+ for (auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
+ I != E; ++I) {
+ if (*I == ".")
+ continue;
+ if (*I == "..") {
+ // Drop the last component.
+ NewPath.resize(llvm::sys::path::parent_path(NewPath).size());
+ } else {
+ if (!NewPath.empty())
+ NewPath += '/';
+ NewPath += *I;
+ }
+ }
+ return NewPath.str();
+}
+
+namespace {
+class HeaderGuardPPCallbacks : public PPCallbacks {
+public:
+ explicit HeaderGuardPPCallbacks(Preprocessor *PP, HeaderGuardCheck *Check)
+ : PP(PP), Check(Check) {}
+
+ void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+ SrcMgr::CharacteristicKind FileType,
+ FileID PrevFID) override {
+ // Record all files we enter. We'll need them to diagnose headers without
+ // guards.
+ SourceManager &SM = PP->getSourceManager();
+ if (Reason == EnterFile && FileType == SrcMgr::C_User) {
+ if (const FileEntry *FE = SM.getFileEntryForID(SM.getFileID(Loc))) {
+ std::string FileName = cleanPath(FE->getName());
+ Files[FileName] = FE;
+ }
+ }
+ }
+
+ void Ifndef(SourceLocation Loc, const Token &MacroNameTok,
+ const MacroDirective *MD) override {
+ if (MD)
+ return;
+
+ // Record #ifndefs that succeeded. We also need the Location of the Name.
+ Ifndefs[MacroNameTok.getIdentifierInfo()] =
+ std::make_pair(Loc, MacroNameTok.getLocation());
+ }
+
+ void MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) override {
+ // Record all Defines macros. We store the whole token to get info on the
+ // Name later.
+ Macros.emplace_back(MacroNameTok, MD);
+ }
+
+ void Endif(SourceLocation Loc, SourceLocation IfLoc) override {
+ // Record all #endif and the corresponding #ifs (including #ifndefs).
+ EndIfs[IfLoc] = Loc;
+ }
+
+ void EndOfMainFile() override {
+ // Now that we have all this information from the preprocessor, use it!
+ SourceManager &SM = PP->getSourceManager();
+
+ for (const auto &MacroEntry : Macros) {
+ const MacroInfo *MI = MacroEntry.second->getMacroInfo();
+
+ // We use clang's header guard detection. This has the advantage of also
+ // emitting a warning for cases where a pseudo header guard is found but
+ // preceeded by something blocking the header guard optimization.
+ if (!MI->isUsedForHeaderGuard())
+ continue;
+
+ const FileEntry *FE =
+ SM.getFileEntryForID(SM.getFileID(MI->getDefinitionLoc()));
+ std::string FileName = cleanPath(FE->getName());
+ Files.erase(FileName);
+
+ // See if we should check and fix this header guard.
+ if (!Check->shouldFixHeaderGuard(FileName))
+ continue;
+
+ // Look up Locations for this guard.
+ SourceLocation ifndef =
+ Ifndefs[MacroEntry.first.getIdentifierInfo()].second;
+ SourceLocation Define = MacroEntry.first.getLocation();
+ SourceLocation EndIf =
+ EndIfs[Ifndefs[MacroEntry.first.getIdentifierInfo()].first];
+
+ // If the macro Name is not equal to what we can compute correct it in the
+ // #ifndef and #define.
+ StringRef Name = MacroEntry.first.getIdentifierInfo()->getName();
+ std::string CPPVar = Check->getHeaderGuard(FileName, Name);
+ std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
+ if (ifndef.isValid() && Name != CPPVar && Name != CPPVarUnder) {
+ Check->diag(ifndef, "header guard does not follow preferred style")
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(
+ ifndef, ifndef.getLocWithOffset(Name.size())),
+ CPPVar)
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(
+ Define, Define.getLocWithOffset(Name.size())),
+ CPPVar);
+ Name = CPPVar;
+ }
+
+ if (!Check->shouldSuggestEndifComment(FileName))
+ continue;
+
+ // Now look at the #endif. We want a comment with the header guard. Fix it
+ // at the slightest deviation.
+ const char *EndIfData = SM.getCharacterData(EndIf);
+ size_t EndIfLen = std::strcspn(EndIfData, "\r\n");
+
+ StringRef EndIfStr(EndIfData, EndIfLen);
+ if (EndIf.isValid() && !EndIfStr.endswith("// " + Name.str())) {
+ std::string Correct = "endif // " + Name.str();
+ Check->diag(EndIf, "#endif for a header guard should reference the "
+ "guard macro in a comment")
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getCharRange(EndIf,
+ EndIf.getLocWithOffset(EndIfLen)),
+ Correct);
+ }
+ }
+
+ // Look for header files that didn't have a header guard. Emit a warning and
+ // fix-its to add the guard.
+ // TODO: Insert the guard after top comments.
+ for (const auto &FE : Files) {
+ StringRef FileName = FE.getKey();
+ if (!Check->shouldSuggestToAddHeaderGuard(FileName))
+ continue;
+
+ FileID FID = SM.translateFile(FE.getValue());
+ SourceLocation StartLoc = SM.getLocForStartOfFile(FID);
+ if (StartLoc.isInvalid())
+ continue;
+
+ std::string CPPVar = Check->getHeaderGuard(FileName);
+ // If there is a header guard macro but it's not in the topmost position
+ // emit a plain warning without fix-its. This often happens when the guard
+ // macro is preceeded by includes.
+ // TODO: Can we move it into the right spot?
+ bool SeenMacro = false;
+ for (const auto &MacroEntry : Macros) {
+ StringRef Name = MacroEntry.first.getIdentifierInfo()->getName();
+ SourceLocation DefineLoc = MacroEntry.first.getLocation();
+ std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
+ if ((Name == CPPVar || Name == CPPVarUnder) &&
+ SM.isWrittenInSameFile(StartLoc, DefineLoc)) {
+ Check->diag(
+ DefineLoc,
+ "Header guard after code/includes. Consider moving it up.");
+ SeenMacro = true;
+ break;
+ }
+ }
+
+ if (SeenMacro)
+ continue;
+
+ Check->diag(StartLoc, "header is missing header guard")
+ << FixItHint::CreateInsertion(
+ StartLoc, "#ifndef " + CPPVar + "\n#define " + CPPVar + "\n\n")
+ << FixItHint::CreateInsertion(SM.getLocForEndOfFile(FID),
+ "\n#endif // " + CPPVar + "\n");
+ }
+ }
+
+private:
+ std::vector<std::pair<Token, const MacroDirective *>> Macros;
+ llvm::StringMap<const FileEntry *> Files;
+ std::map<const IdentifierInfo *, std::pair<SourceLocation, SourceLocation>>
+ Ifndefs;
+ std::map<SourceLocation, SourceLocation> EndIfs;
+
+ Preprocessor *PP;
+ HeaderGuardCheck *Check;
+};
+} // namespace
+
+void HeaderGuardCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+ Compiler.getPreprocessor().addPPCallbacks(
+ new HeaderGuardPPCallbacks(&Compiler.getPreprocessor(), this));
+}
+
+bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
+ return FileName.endswith(".h");
+}
+
+bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
+
+bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
+ return FileName.endswith(".h");
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/utils/HeaderGuard.h
===================================================================
--- /dev/null
+++ clang-tidy/utils/HeaderGuard.h
@@ -0,0 +1,41 @@
+//===--- HeaderGuard.h - clang-tidy -----------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_GUARD_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_GUARD_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+// Finds and fixes header guards.
+class HeaderGuardCheck : public ClangTidyCheck {
+public:
+ void registerPPCallbacks(CompilerInstance &Compiler) override;
+
+ /// \brief return true if the checker should suggest inserting a trailing
+ /// comment on the #endif of the header guard. It will use the same name as
+ /// returned by getHeaderGuard.
+ virtual bool shouldSuggestEndifComment(StringRef Filename);
+ /// \brief return true if the checker should suggest changing a header guard
+ /// the string returned by getHeaderGuard.
+ virtual bool shouldFixHeaderGuard(StringRef Filename);
+ /// \brief return true if the checker should add a header guard to the file if
+ /// it has none.
+ virtual bool shouldSuggestToAddHeaderGuard(StringRef Filename);
+ /// \brief Get the canonical header guard for a file.
+ virtual std::string getHeaderGuard(StringRef Filename,
+ StringRef OldGuard = StringRef()) = 0;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_GUARD_H
Index: unittests/clang-tidy/LLVMModuleTest.cpp
===================================================================
--- unittests/clang-tidy/LLVMModuleTest.cpp
+++ unittests/clang-tidy/LLVMModuleTest.cpp
@@ -1,4 +1,5 @@
#include "ClangTidyTest.h"
+#include "llvm/HeaderGuardCheck.h"
#include "llvm/IncludeOrderCheck.h"
#include "llvm/NamespaceCommentCheck.h"
#include "gtest/gtest.h"
@@ -85,6 +86,20 @@
"} // namespace asdf"));
}
+static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename) {
+ return test::runCheckOnCode<LLVMHeaderGuardCheck>(
+ Code, /*Errors=*/nullptr, Filename, std::string("-xc++-header"));
+}
+
+TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
+ EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif\n",
+ runHeaderGuardCheck("#ifndef FOO\n#define FOO\n#endif\n",
+ "include/llvm/ADT/foo.h"));
+
+ EXPECT_EQ("#ifndef LLVM_CLANG_C_BAR_H\n#define LLVM_CLANG_C_BAR_H\n#endif\n",
+ runHeaderGuardCheck("", "./include/clang-c/bar.h"));
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
