Flip the order the preprocessor and frontendaction are informed of the end of a file.

[bkramer]

Summary

This allows using EndOfMainFile from a PPCallback to access data from the
action. The pattern of PPCallback referencing an action is common in clang-tidy.

[klimek]

I'm fine with it - we might want to ask Richard whether he can imagine any problems with this change...

[rsmith]

This seems like a more natural order for these actions to me. Can you add a test? (Install a preprocessing callback and check that its EndOfMainFile action is called before the FrontendAction's EndSourceFileAction)

[bkramer]

Added unit test.

[rsmith]

LGTM

[bkramer]

Closed by commit rL215145 (authored by d0k).

cfe/trunk/lib/Frontend/FrontendAction.cpp

 }
 
 void FrontendAction::EndSourceFile() {
   CompilerInstance &CI = getCompilerInstance();
 
   // Inform the diagnostic client we are done with this source file.
   CI.getDiagnosticClient().EndSourceFile();
 
+  // Inform the preprocessor we are done.
+  if (CI.hasPreprocessor())
+    CI.getPreprocessor().EndSourceFile();
+
   // Finalize the action.
   EndSourceFileAction();
 
   // Sema references the ast consumer, so reset sema first.
   //
   // FIXME: There is more per-file stuff we could just drop here?
   bool DisableFree = CI.getFrontendOpts().DisableFree;
   if (DisableFree) {
     if (!isCurrentFileAST()) {
       CI.resetAndLeakSema();
       CI.resetAndLeakASTContext();
     }
     BuryPointer(CI.takeASTConsumer());
   } else {
     if (!isCurrentFileAST()) {
       CI.setSema(nullptr);
       CI.setASTContext(nullptr);
     }
     CI.setASTConsumer(nullptr);
   }
 
-  // Inform the preprocessor we are done.
-  if (CI.hasPreprocessor())
-    CI.getPreprocessor().EndSourceFile();
-
   if (CI.getFrontendOpts().ShowStats) {
     llvm::errs() << "\nSTATISTICS FOR '" << getCurrentFile() << "':\n";
     CI.getPreprocessor().PrintStats();
     CI.getPreprocessor().getIdentifierTable().PrintStats();
     CI.getPreprocessor().getHeaderSearchInfo().PrintStats();
     CI.getSourceManager().PrintStats();
     llvm::errs() << "\n";
   }

cfe/trunk/unittests/Frontend/FrontendActionTest.cpp

TEST(ASTFrontendAction, IncrementalParsing) {
 
   TestASTFrontendAction test_action(/*enableIncrementalProcessing=*/true);
   ASSERT_TRUE(compiler.ExecuteAction(test_action));
   ASSERT_EQ(2U, test_action.decl_names.size());
   EXPECT_EQ("main", test_action.decl_names[0]);
   EXPECT_EQ("x", test_action.decl_names[1]);
 }
 
+struct TestPPCallbacks : public PPCallbacks {
+  TestPPCallbacks() : SeenEnd(false) {}
+
+  void EndOfMainFile() override { SeenEnd = true; }
+
+  bool SeenEnd;
+};
+
+class TestPPCallbacksFrontendAction : public PreprocessorFrontendAction {
+  TestPPCallbacks *Callbacks;
+
+public:
+  TestPPCallbacksFrontendAction(TestPPCallbacks *C)
+      : Callbacks(C), SeenEnd(false) {}
+
+  void ExecuteAction() override {
+    Preprocessor &PP = getCompilerInstance().getPreprocessor();
+    PP.addPPCallbacks(Callbacks);
+    PP.EnterMainSourceFile();
+  }
+  void EndSourceFileAction() override { SeenEnd = Callbacks->SeenEnd; }
+
+  bool SeenEnd;
+};
+
+TEST(PreprocessorFrontendAction, EndSourceFile) {
+  CompilerInvocation *Invocation = new CompilerInvocation;
+  Invocation->getPreprocessorOpts().addRemappedFile(
+      "test.cc", MemoryBuffer::getMemBuffer("int main() { float x; }"));
+  Invocation->getFrontendOpts().Inputs.push_back(
+      FrontendInputFile("test.cc", IK_CXX));
+  Invocation->getFrontendOpts().ProgramAction = frontend::ParseSyntaxOnly;
+  Invocation->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+  CompilerInstance Compiler;
+  Compiler.setInvocation(Invocation);
+  Compiler.createDiagnostics();
+
+  TestPPCallbacks *Callbacks = new TestPPCallbacks;
+  TestPPCallbacksFrontendAction TestAction(Callbacks);
+  ASSERT_FALSE(Callbacks->SeenEnd);
+  ASSERT_FALSE(TestAction.SeenEnd);
+  ASSERT_TRUE(Compiler.ExecuteAction(TestAction));
+  // Check that EndOfMainFile was called before EndSourceFileAction.
+  ASSERT_TRUE(TestAction.SeenEnd);
+}
+
 } // anonymous namespace