salman-javed-nz created this revision.
salman-javed-nz added reviewers: whisperity, hokein, aaron.ballman.
salman-javed-nz added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, rnkovacs, xazax.hun.
salman-javed-nz requested review of this revision.
Fixes https://bugs.llvm.org/show_bug.cgi?id=48613.
llvm-header-guard is suggesting header guards with leading underscores
if the header file path begins with a '/' or similar special character.
Only reserved identifiers should begin with an underscore.
Another problem is that llvm-header-guard does the character
substitution for '/', '.', and '-' only.
There are other characters which are valid in a file path but invalid
in a macro name. Rely on the functions provided in CharInfo.h to
identify those characters.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D114149
Files:
clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
clang-tools-extra/clang-tidy/utils/HeaderGuard.h
clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -219,6 +219,25 @@
"", "/llvm-project/clang-tools-extra/clangd/foo.h",
StringRef("header is missing header guard")));
+ // Characters outside [A-z0-9] are replaced with "_".
+ EXPECT_EQ("#ifndef FOO_BAR_TE_T_A_B_C_H\n"
+ "#define FOO_BAR_TE_T_A_B_C_H\n"
+ "\n"
+ "\n"
+ "#endif\n",
+ runHeaderGuardCheck("", "include/foo-bar/te$t/a(b)c.h",
+ StringRef("header is missing header guard")));
+
+ // Subtitution of characters should not result in a header guard starting
+ // with "_".
+ EXPECT_EQ("#ifndef BAR_H\n"
+ "#define BAR_H\n"
+ "\n"
+ "\n"
+ "#endif\n",
+ runHeaderGuardCheck("", "include/$bar.h",
+ StringRef("header is missing header guard")));
+
#ifdef WIN32
// Check interaction with Windows-style path separators (\).
EXPECT_EQ(
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -38,6 +38,9 @@
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
+ /// Ensure that the provided header guard is a valid identifier.
+ static std::string sanitizeHeaderGuard(StringRef Guard);
+
/// Returns ``true`` if the check should suggest inserting a trailing comment
/// on the ``#endif`` of the header guard. It will use the same name as
/// returned by ``HeaderGuardCheck::getHeaderGuard``.
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -24,6 +24,27 @@
return std::string(Result.str());
}
+/// Replace invalid characters in the identifier with '_'.
+static std::string replaceInvalidChars(StringRef Identifier) {
+ std::string Fixed{Identifier};
+ for (auto It = Fixed.begin(); It != Fixed.end(); ++It) {
+ // Identifier name must start with [A-z].
+ if (It == Fixed.begin() && isAsciiIdentifierStart(*It))
+ continue;
+ // The subsequent characters can be [A-z0-9].
+ if (It > Fixed.begin() && isAsciiIdentifierContinue(*It))
+ continue;
+ *It = '_';
+ }
+ return Fixed;
+}
+
+/// Remove all '_' characters at the beginning of the identifier. Only reserved
+/// identifiers are allowed to start with these.
+static StringRef dropLeadingUnderscores(StringRef Identifier) {
+ return Identifier.drop_while([](char c) { return c == '_'; });
+}
+
namespace {
class HeaderGuardPPCallbacks : public PPCallbacks {
public:
@@ -164,6 +185,7 @@
StringRef CurHeaderGuard,
std::vector<FixItHint> &FixIts) {
std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard);
+ CPPVar = HeaderGuardCheck::sanitizeHeaderGuard(CPPVar);
std::string CPPVarUnder = CPPVar + '_';
// Allow a trailing underscore iff we don't have to change the endif comment
@@ -216,6 +238,7 @@
continue;
std::string CPPVar = Check->getHeaderGuard(FileName);
+ CPPVar = HeaderGuardCheck::sanitizeHeaderGuard(CPPVar);
std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
// If there's a macro with a name that follows the header guard convention
// but was not recognized by the preprocessor as a header guard there must
@@ -278,6 +301,11 @@
PP->addPPCallbacks(std::make_unique<HeaderGuardPPCallbacks>(PP, this));
}
+std::string HeaderGuardCheck::sanitizeHeaderGuard(StringRef Guard) {
+ std::string Sanitized = replaceInvalidChars(Guard);
+ return dropLeadingUnderscores(Sanitized).str();
+}
+
bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
return utils::isFileExtension(FileName, HeaderFileExtensions);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
