[PATCH] Header guard warning is too agressive

[Ismail Pazarbasi]

Hi rsmith, rafael.espindola,

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, or it might be defining something completely different. This can be observed in the wild when handling feature macros or header guards in different files.

// foo.c
#ifndef NO_FLOATING_POINT_SUPPORT
#define USE_SOMETHING_ELSE
#include "foo.h"
/* ... */
#endif /* !defined(NO_FLOATING_POINT_SUPPORT)

http://llvm-reviews.chandlerc.com/D1506

Files:
  lib/Lex/PPLexerChange.cpp
  test/Lexer/header.cpp
  test/Lexer/Inputs/unlikely-to-be-header-guard.h

Index: lib/Lex/PPLexerChange.cpp
===================================================================
--- lib/Lex/PPLexerChange.cpp
+++ lib/Lex/PPLexerChange.cpp
@@ -251,19 +251,36 @@
           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/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]*}}:
Index: test/Lexer/Inputs/unlikely-to-be-header-guard.h
===================================================================
--- /dev/null
+++ test/Lexer/Inputs/unlikely-to-be-header-guard.h
@@ -0,0 +1,4 @@
+#ifndef data_rep_h
+#define use_alternate_data_rep
+/* #include "data_rep.h" */
+#endif
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1506.1.patch
Type: text/x-patch
Size: 3521 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130824/b3277fc4/attachment.bin>