salman-javed-nz updated this revision to Diff 388699.
salman-javed-nz edited the summary of this revision.
salman-javed-nz added a comment.
Back out the "replace invalid characters in an identifier with underscores"
feature.
Making this feature work with Unicode characters on different operating systems
significantly increases the amount of code change required.
This can always be revisited in another patch.
In this patch, let's focus on fixing the most pressing problem at hand first:
header guards starting with underscores.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114149/new/
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,16 @@
"", "/llvm-project/clang-tools-extra/clangd/foo.h",
StringRef("header is missing header guard")));
+ // Substitution 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 non-reserved identifier.
+ 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
@@ -164,9 +164,10 @@
StringRef CurHeaderGuard,
std::vector<FixItHint> &FixIts) {
std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard);
+ CPPVar = Check->sanitizeHeaderGuard(CPPVar);
std::string CPPVarUnder = CPPVar + '_';
- // Allow a trailing underscore iff we don't have to change the endif
comment
+ // Allow a trailing underscore if we don't have to change the endif comment
// too.
if (Ifndef.isValid() && CurHeaderGuard != CPPVar &&
(CurHeaderGuard != CPPVarUnder ||
@@ -216,6 +217,7 @@
continue;
std::string CPPVar = Check->getHeaderGuard(FileName);
+ CPPVar = Check->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 +280,11 @@
PP->addPPCallbacks(std::make_unique<HeaderGuardPPCallbacks>(PP, this));
}
+std::string HeaderGuardCheck::sanitizeHeaderGuard(StringRef Guard) {
+ // Only reserved identifiers are allowed to start with an '_'.
+ return Guard.drop_while([](char C) { return C == '_'; }).str();
+}
+
bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
return utils::isFileExtension(FileName, HeaderFileExtensions);
}
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,16 @@
"", "/llvm-project/clang-tools-extra/clangd/foo.h",
StringRef("header is missing header guard")));
+ // Substitution 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 non-reserved identifier.
+ 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
@@ -164,9 +164,10 @@
StringRef CurHeaderGuard,
std::vector<FixItHint> &FixIts) {
std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard);
+ CPPVar = Check->sanitizeHeaderGuard(CPPVar);
std::string CPPVarUnder = CPPVar + '_';
- // Allow a trailing underscore iff we don't have to change the endif comment
+ // Allow a trailing underscore if we don't have to change the endif comment
// too.
if (Ifndef.isValid() && CurHeaderGuard != CPPVar &&
(CurHeaderGuard != CPPVarUnder ||
@@ -216,6 +217,7 @@
continue;
std::string CPPVar = Check->getHeaderGuard(FileName);
+ CPPVar = Check->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 +280,11 @@
PP->addPPCallbacks(std::make_unique<HeaderGuardPPCallbacks>(PP, this));
}
+std::string HeaderGuardCheck::sanitizeHeaderGuard(StringRef Guard) {
+ // Only reserved identifiers are allowed to start with an '_'.
+ return Guard.drop_while([](char C) { return C == '_'; }).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
