[PATCH] D20858: [clang-format] make header guard identification stricter in header insertion.

[Eric Liu via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: djasper.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

don't just identify first #define as header guard anymore. Use #ifndef
#define to tell if a #define belongs to header guard.

http://reviews.llvm.org/D20858

Files:
  lib/Format/Format.cpp
  unittests/Format/CleanupTest.cpp

Index: unittests/Format/CleanupTest.cpp
===================================================================
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -537,6 +537,34 @@
   EXPECT_EQ(Expected, formatAndApply(Code, Replaces));
 }
 
+TEST_F(CleanUpReplacementsTest, DontJustInsertAfterRandomDefine) {
+  std::string Code = "#define X 1";
+  std::string Expected = "#include <vector>\n"
+                         "#define X 1";
+  tooling::Replacements Replaces = {createInsertion("#include <vector>")};
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, InsertAfterHeaderGuard) {
+  std::string Code = "#ifndef X_H\n"
+                     "#define X_H\n";
+  std::string Expected = "#ifndef X_H\n"
+                         "#define X_H\n"
+                         "#include <vector>\n";
+  tooling::Replacements Replaces = {createInsertion("#include <vector>")};
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, DontInsertAfterNonMatchedHeaderGuard) {
+  std::string Code = "#ifndef X_H\n"
+                     "#define Y_H";
+  std::string Expected = "#include <vector>\n"
+                         "#ifndef X_H\n"
+                         "#define Y_H";
+  tooling::Replacements Replaces = {createInsertion("#include <vector>")};
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1469,7 +1469,8 @@
                       std::inserter(Result, Result.begin()));
 
   llvm::Regex IncludeRegex(IncludeRegexPattern);
-  llvm::Regex DefineRegex(R"(^[\t\ ]*#[\t\ ]*define[\t\ ]*[^\\]*$)");
+  llvm::Regex IfNDefRegex(R"(^[\t\ ]*#[\t\ ]*ifndef[\t\ ]*([^\\]*)$)");
+  llvm::Regex DefineRegex(R"(^[\t\ ]*#[\t\ ]*define[\t\ ]*([^\\]*)$)");
   SmallVector<StringRef, 4> Matches;
 
   StringRef FileName = Replaces.begin()->getFilePath();
@@ -1484,7 +1485,9 @@
     Priorities.insert(Category.Priority);
   int FirstIncludeOffset = -1;
   int Offset = 0;
+  int AfterFirstIfNDef = 0;
   int AfterHeaderGuard = 0;
+  StringRef FirstIfNDefName;
   SmallVector<StringRef, 32> Lines;
   Code.split(Lines, '\n');
   for (auto Line : Lines) {
@@ -1496,10 +1499,15 @@
       if (FirstIncludeOffset < 0)
         FirstIncludeOffset = Offset;
     }
+    if (AfterHeaderGuard == 0 && AfterFirstIfNDef > 0 &&
+        AfterFirstIfNDef == Offset && DefineRegex.match(Line, &Matches) &&
+        Matches[1] == FirstIfNDefName)
+      AfterHeaderGuard = Offset + Line.size() + 1;
     Offset += Line.size() + 1;
-    // FIXME: make header guard matching stricter, e.g. consider #ifndef.
-    if (AfterHeaderGuard == 0 && DefineRegex.match(Line))
-      AfterHeaderGuard = Offset;
+    if (AfterFirstIfNDef == 0 && IfNDefRegex.match(Line, &Matches)) {
+      FirstIfNDefName = Matches[1];
+      AfterFirstIfNDef = Offset;
+    }
   }
 
   // Populate CategoryEndOfssets:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D20858.59199.patch
Type: text/x-patch
Size: 3082 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160601/92bee898/attachment.bin>