[PATCH] Header guard warning is too agressive
Ismail Pazarbasi
ismail.pazarbasi at gmail.com
Fri Oct 11 02:54:57 PDT 2013
Except line 277, (const size_t MaxHalfLength = std::max...), it's the output from clang-format within range 268:296. Update addresses Richard's comments.
I have seen this warning fired in legacy code. Our case is quite similar to Ted's example. PR17053 is filed after I submitted this. Please don't consider this patch as a fix for PR17053. It seems like this patch, as Richard said previously, coincidentally fixes PR17053 as well.
Hi rafael.espindola,
http://llvm-reviews.chandlerc.com/D1506
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1506?vs=3738&id=4830#toc
Files:
lib/Lex/PPLexerChange.cpp
test/Lexer/Inputs/unlikely-to-be-header-guard.h
test/Lexer/header.cpp
Index: lib/Lex/PPLexerChange.cpp
===================================================================
--- lib/Lex/PPLexerChange.cpp
+++ lib/Lex/PPLexerChange.cpp
@@ -264,19 +264,32 @@
if (!ControllingMacro->hasMacroDefinition() &&
DefinedMacro != ControllingMacro &&
HeaderInfo.FirstTimeLexingFile(FE)) {
- // Emit a warning for a bad header guard.
- Diag(CurPPLexer->MIOpt.GetMacroLocation(),
- diag::warn_header_guard)
- << CurPPLexer->MIOpt.GetMacroLocation()
- << ControllingMacro;
- Diag(CurPPLexer->MIOpt.GetDefinedLocation(),
- diag::note_header_guard)
- << CurPPLexer->MIOpt.GetDefinedLocation()
- << DefinedMacro
- << ControllingMacro
- << FixItHint::CreateReplacement(
- CurPPLexer->MIOpt.GetDefinedLocation(),
- ControllingMacro->getName());
+
+ // If the edit distance between the two macros is more than 50%,
+ // DefinedMacro may not be header guard, or can be header guard of
+ // another header file. Therefore, it maybe defining something
+ // completely different. This can be observed in the wild when
+ // handling feature macros or header guards in different files.
+
+ const StringRef ControllingMacroName = ControllingMacro->getName();
+ const StringRef DefinedMacroName = DefinedMacro->getName();
+ const size_t MaxHalfLength = std::max(ControllingMacroName.size(),
+ DefinedMacroName.size()) / 2;
+ const unsigned ED = ControllingMacroName.edit_distance(
+ DefinedMacroName, true, MaxHalfLength);
+ if (ED <= MaxHalfLength) {
+ // Emit a warning for a bad header guard.
+ Diag(CurPPLexer->MIOpt.GetMacroLocation(),
+ diag::warn_header_guard)
+ << CurPPLexer->MIOpt.GetMacroLocation() << ControllingMacro;
+ Diag(CurPPLexer->MIOpt.GetDefinedLocation(),
+ diag::note_header_guard)
+ << CurPPLexer->MIOpt.GetDefinedLocation() << DefinedMacro
+ << ControllingMacro
+ << FixItHint::CreateReplacement(
+ CurPPLexer->MIOpt.GetDefinedLocation(),
+ ControllingMacro->getName());
+ }
}
}
}
Index: test/Lexer/Inputs/unlikely-to-be-header-guard.h
===================================================================
--- /dev/null
+++ test/Lexer/Inputs/unlikely-to-be-header-guard.h
@@ -0,0 +1,5 @@
+#ifndef data_rep_h
+#define use_alternate_data_rep
+/* #include "data_rep.h" */
+#endif
+
Index: test/Lexer/header.cpp
===================================================================
--- test/Lexer/header.cpp
+++ test/Lexer/header.cpp
@@ -6,6 +6,7 @@
#include "Inputs/different-define.h"
#include "Inputs/out-of-order-define.h"
#include "Inputs/tokens-between-ifndef-and-define.h"
+#include "Inputs/unlikely-to-be-header-guard.h"
#include "Inputs/bad-header-guard.h"
// CHECK: In file included from {{.*}}header.cpp:{{[0-9]*}}:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1506.2.patch
Type: text/x-patch
Size: 3292 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131011/380a7cfb/attachment.bin>
More information about the cfe-commits
mailing list