Make DeletedLines local variables in checkEmptyNamespace.

[ioeric]

Summary

Patch by Sam McCall!

[djasper]

I think we should add a test for why this was a problem. IIUC, this is caused by something like:

#if a
#else
#endif
namespace {
}

Because we actually analyze the whole thing twice.

[ioeric]

• Added a test case.

[djasper]

Looks good.

cfe/trunk/lib/Format/Format.cpp

for (FormatToken *Tok = Line.First; Tok != nullptr; Tok = Tok->Next) {
       if (Tok->isNot(tok::comment))
         return false;
     }
     return true;
   }
 
   // Iterate through all lines and remove any empty (nested) namespaces.
   void checkEmptyNamespace(SmallVectorImpl<AnnotatedLine *> &AnnotatedLines) {
+    std::set<unsigned> DeletedLines;
     for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
       auto &Line = *AnnotatedLines[i];
       if (Line.startsWith(tok::kw_namespace) ||
           Line.startsWith(tok::kw_inline, tok::kw_namespace)) {
-        checkEmptyNamespace(AnnotatedLines, i, i);
+        checkEmptyNamespace(AnnotatedLines, i, i, DeletedLines);
       }
     }
 
     for (auto Line : DeletedLines) {
       FormatToken *Tok = AnnotatedLines[Line]->First;
       while (Tok) {
         deleteToken(Tok);
         Tok = Tok->Next;
       }
     }
   }
 
   // The function checks if the namespace, which starts from \p CurrentLine, and
   // its nested namespaces are empty and delete them if they are empty. It also
   // sets \p NewLine to the last line checked.
   // Returns true if the current namespace is empty.
   bool checkEmptyNamespace(SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
-                           unsigned CurrentLine, unsigned &NewLine) {
+                           unsigned CurrentLine, unsigned &NewLine,
+                           std::set<unsigned> &DeletedLines) {
     unsigned InitLine = CurrentLine, End = AnnotatedLines.size();
     if (Style.BraceWrapping.AfterNamespace) {
       // If the left brace is in a new line, we should consume it first so that
       // it does not make the namespace non-empty.
       // FIXME: error handling if there is no left brace.
       if (!AnnotatedLines[++CurrentLine]->startsWith(tok::l_brace)) {
         NewLine = CurrentLine;
         return false;
       }
     } else if (!AnnotatedLines[CurrentLine]->endsWith(tok::l_brace)) {
       return false;
     }
     while (++CurrentLine < End) {
       if (AnnotatedLines[CurrentLine]->startsWith(tok::r_brace))
         break;
 
       if (AnnotatedLines[CurrentLine]->startsWith(tok::kw_namespace) ||
           AnnotatedLines[CurrentLine]->startsWith(tok::kw_inline,
                                                   tok::kw_namespace)) {
-        if (!checkEmptyNamespace(AnnotatedLines, CurrentLine, NewLine))
+        if (!checkEmptyNamespace(AnnotatedLines, CurrentLine, NewLine,
+                                 DeletedLines))
           return false;
         CurrentLine = NewLine;
         continue;
       }
 
       if (containsOnlyComments(*AnnotatedLines[CurrentLine]))
         continue;
 
bool operator()(const FormatToken *LHS, const FormatToken *RHS) const {
       return SM.isBeforeInTranslationUnit(LHS->Tok.getLocation(),
                                           RHS->Tok.getLocation());
     }
     const SourceManager &SM;
   };
 
   // Tokens to be deleted.
   std::set<FormatToken *, FormatTokenLess> DeletedTokens;
-  // The line numbers of lines to be deleted.
-  std::set<unsigned> DeletedLines;
 };
 
 struct IncludeDirective {
   StringRef Filename;
   StringRef Text;
   unsigned Offset;
   int Category;
 };

cfe/trunk/unittests/Format/CleanupTest.cpp

TEST_F(CleanupTest, EmptyNamespaceWithCommentsBreakBeforeBrace) {
   std::string Expected = "\n\n\n\n\n\n\n\n\n\n";
   std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
   FormatStyle Style = getLLVMStyle();
   Style.BraceWrapping.AfterNamespace = true;
   std::string Result = cleanup(Code, Ranges, Style);
   EXPECT_EQ(Expected, Result);
 }
 
+TEST_F(CleanupTest, EmptyNamespaceAroundConditionalCompilation) {
+  std::string Code = "#ifdef A\n"
+                     "int a;\n"
+                     "int b;\n"
+                     "#else\n"
+                     "#endif\n"
+                     "namespace {}";
+  std::string Expected = "#ifdef A\n"
+                         "int a;\n"
+                         "int b;\n"
+                         "#else\n"
+                         "#endif\n";
+  std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
+  FormatStyle Style = getLLVMStyle();
+  std::string Result = cleanup(Code, Ranges, Style);
+  EXPECT_EQ(Expected, Result);
+}
+
 TEST_F(CleanupTest, CtorInitializationSimpleRedundantComma) {
   std::string Code = "class A {\nA() : , {} };";
   std::string Expected = "class A {\nA()  {} };";
   EXPECT_EQ(Expected, cleanupAroundOffsets({17, 19}, Code));
 
   Code = "class A {\nA() : x(1), {} };";
   Expected = "class A {\nA() : x(1) {} };";
   EXPECT_EQ(Expected, cleanupAroundOffsets({23}, Code));