sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Currently it rejects "// FOO_BAR_H" as an endif comment due to the extra space.
A user complained that this is too picky, which seems fair enough.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124955
Files:
clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
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
@@ -204,6 +204,14 @@
"include/llvm/ADT/foo.h",
StringRef("header guard does not follow preferred style")));
+ // An extra space inside the comment is OK.
+ llvm::StringRef WithExtraSpace = "#ifndef LLVM_ADT_FOO_H\n"
+ "#define LLVM_ADT_FOO_H\n"
+ "#endif // LLVM_ADT_FOO_H\n";
+ EXPECT_EQ(WithExtraSpace,
+ runHeaderGuardCheckWithEndif(WithExtraSpace,
+ "include/llvm/ADT/foo.h", None));
+
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif \\ \n"
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
@@ -145,13 +145,13 @@
EndIfStr[FindEscapedNewline] == '\\')
return false;
- if (!Check->shouldSuggestEndifComment(FileName) &&
- !(EndIfStr.startswith("//") ||
- (EndIfStr.startswith("/*") && EndIfStr.endswith("*/"))))
- return false;
+ bool IsLineComment =
+ EndIfStr.consume_front("//") ||
+ (EndIfStr.consume_front("/*") && EndIfStr.consume_back("*/"));
+ if (!IsLineComment)
+ return Check->shouldSuggestEndifComment(FileName);
- return (EndIfStr != "// " + HeaderGuard.str()) &&
- (EndIfStr != "/* " + HeaderGuard.str() + " */");
+ return EndIfStr.trim() != HeaderGuard;
}
/// Look for header guards that don't match the preferred style. Emit
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
@@ -204,6 +204,14 @@
"include/llvm/ADT/foo.h",
StringRef("header guard does not follow preferred style")));
+ // An extra space inside the comment is OK.
+ llvm::StringRef WithExtraSpace = "#ifndef LLVM_ADT_FOO_H\n"
+ "#define LLVM_ADT_FOO_H\n"
+ "#endif // LLVM_ADT_FOO_H\n";
+ EXPECT_EQ(WithExtraSpace,
+ runHeaderGuardCheckWithEndif(WithExtraSpace,
+ "include/llvm/ADT/foo.h", None));
+
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
"#define LLVM_ADT_FOO_H\n"
"#endif \\ \n"
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
@@ -145,13 +145,13 @@
EndIfStr[FindEscapedNewline] == '\\')
return false;
- if (!Check->shouldSuggestEndifComment(FileName) &&
- !(EndIfStr.startswith("//") ||
- (EndIfStr.startswith("/*") && EndIfStr.endswith("*/"))))
- return false;
+ bool IsLineComment =
+ EndIfStr.consume_front("//") ||
+ (EndIfStr.consume_front("/*") && EndIfStr.consume_back("*/"));
+ if (!IsLineComment)
+ return Check->shouldSuggestEndifComment(FileName);
- return (EndIfStr != "// " + HeaderGuard.str()) &&
- (EndIfStr != "/* " + HeaderGuard.str() + " */");
+ return EndIfStr.trim() != HeaderGuard;
}
/// Look for header guards that don't match the preferred style. Emit
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
