This was the sort of question I had originally: is a begin callback useful enough to be put in Tooling code? Then I thought: if an end callback is available it only makes sense to have a begin callback for symmetry. Otherwise, one day someone will come along and, like me, say: "Here's an end callback but no begin callback but it's the begin callback I really want. Why is it missing?"
If the callback is a problem I'll remove it and add a custom FrontendAction where it's needed in the migrator. > -----Original Message----- > From: Manuel Klimek [mailto:[email protected]] > Sent: Monday, June 03, 2013 8:59 AM > To: Vane, Edwin > Cc: [email protected] > Subject: Re: r182864 - Tooling: Call back for both begin and end of sources > > Hmm... My main concern is whether this is really common enough that we don't > want to just write FrontendActions for where we need it (it's not hard to > write > your own FrontendAction - the methods in tooling are mainly there to make the > very common use cases less code to write). > > > On Wed, May 29, 2013 at 6:01 PM, Edwin Vane <[email protected]> wrote: > > > Author: revane > Date: Wed May 29 11:01:10 2013 > New Revision: 182864 > > URL: http://llvm.org/viewvc/llvm-project?rev=182864&view=rev > Log: > Tooling: Call back for both begin and end of sources > > newFrontendActionFactory() took a pointer to a callback to call when a > source > file was done being processed by an action. This revision updates the > callback > to include an ante-processing callback as well. > > Callback-providing class renamed and callback functions themselves > renamed. > Functions are no longer pure-virtual so users aren't forced to implement > both > callbacks if one isn't needed. > > > Modified: > cfe/trunk/include/clang/Tooling/Tooling.h > cfe/trunk/unittests/Tooling/ToolingTest.cpp > > Modified: cfe/trunk/include/clang/Tooling/Tooling.h > URL: http://llvm.org/viewvc/llvm- > project/cfe/trunk/include/clang/Tooling/Tooling.h?rev=182864&r1=182863&r2 > =182864&view=diff > ========================================================== > ==================== > --- cfe/trunk/include/clang/Tooling/Tooling.h (original) > +++ cfe/trunk/include/clang/Tooling/Tooling.h Wed May 29 11:01:10 > 2013 > @@ -74,12 +74,22 @@ public: > template <typename T> > FrontendActionFactory *newFrontendActionFactory(); > > -/// \brief Called at the end of each source file when used with > -/// \c newFrontendActionFactory. > -class EndOfSourceFileCallback { > +/// \brief Callbacks called before and after each source file processed > by a > +/// FrontendAction created by the FrontedActionFactory returned by \c > +/// newFrontendActionFactory. > +class SourceFileCallbacks { > public: > - virtual ~EndOfSourceFileCallback() {} > - virtual void run() = 0; > + virtual ~SourceFileCallbacks() {} > + > + /// \brief Called before a source file is processed by a > FrontEndAction. > > > > FrontendAction. > > > + /// \see clang::FrontendAction::BeginSourceFileAction > + virtual bool BeginSource(CompilerInstance &CI, StringRef Filename) { > + return true; > + } > + > + /// \brief Called after a source file is processed by a > FrontendAction. > + /// \see clang::FrontendAction::EndSourceFileAction > + virtual void EndSource() {} > }; > > /// \brief Returns a new FrontendActionFactory for any type that > provides an > @@ -95,7 +105,7 @@ public: > /// newFrontendActionFactory(&Factory); > template <typename FactoryT> > inline FrontendActionFactory *newFrontendActionFactory( > - FactoryT *ConsumerFactory, EndOfSourceFileCallback *EndCallback > = NULL); > + FactoryT *ConsumerFactory, SourceFileCallbacks *Callbacks = > NULL); > > /// \brief Runs (and deletes) the tool on 'Code' with the -fsyntax-only > flag. > /// > @@ -226,23 +236,23 @@ FrontendActionFactory *newFrontendAction > > template <typename FactoryT> > inline FrontendActionFactory *newFrontendActionFactory( > - FactoryT *ConsumerFactory, EndOfSourceFileCallback *EndCallback) > { > + FactoryT *ConsumerFactory, SourceFileCallbacks *Callbacks) { > class FrontendActionFactoryAdapter : public FrontendActionFactory { > public: > explicit FrontendActionFactoryAdapter(FactoryT *ConsumerFactory, > - EndOfSourceFileCallback > *EndCallback) > - : ConsumerFactory(ConsumerFactory), EndCallback(EndCallback) {} > + SourceFileCallbacks > *Callbacks) > + : ConsumerFactory(ConsumerFactory), Callbacks(Callbacks) {} > > virtual clang::FrontendAction *create() { > - return new ConsumerFactoryAdaptor(ConsumerFactory, > EndCallback); > + return new ConsumerFactoryAdaptor(ConsumerFactory, Callbacks); > } > > private: > class ConsumerFactoryAdaptor : public clang::ASTFrontendAction { > public: > ConsumerFactoryAdaptor(FactoryT *ConsumerFactory, > - EndOfSourceFileCallback *EndCallback) > - : ConsumerFactory(ConsumerFactory), EndCallback(EndCallback) {} > + SourceFileCallbacks *Callbacks) > + : ConsumerFactory(ConsumerFactory), Callbacks(Callbacks) {} > > clang::ASTConsumer *CreateASTConsumer(clang::CompilerInstance > &, > StringRef) { > @@ -250,21 +260,29 @@ inline FrontendActionFactory *newFronten > } > > protected: > - virtual void EndSourceFileAction() { > - if (EndCallback != NULL) > - EndCallback->run(); > + virtual bool BeginSourceFileAction(CompilerInstance &CI, > + StringRef Filename) > LLVM_OVERRIDE { > + if (!clang::ASTFrontendAction::BeginSourceFileAction(CI, > Filename)) > + return false; > + if (Callbacks != NULL) > + return Callbacks->BeginSource(CI, Filename); > + return true; > + } > + virtual void EndSourceFileAction() LLVM_OVERRIDE { > + if (Callbacks != NULL) > + Callbacks->EndSource(); > clang::ASTFrontendAction::EndSourceFileAction(); > } > > private: > FactoryT *ConsumerFactory; > - EndOfSourceFileCallback *EndCallback; > + SourceFileCallbacks *Callbacks; > }; > FactoryT *ConsumerFactory; > - EndOfSourceFileCallback *EndCallback; > + SourceFileCallbacks *Callbacks; > }; > > - return new FrontendActionFactoryAdapter(ConsumerFactory, > EndCallback); > + return new FrontendActionFactoryAdapter(ConsumerFactory, > Callbacks); > } > > /// \brief Returns the absolute path of \c File, by prepending it with > > Modified: cfe/trunk/unittests/Tooling/ToolingTest.cpp > URL: http://llvm.org/viewvc/llvm- > project/cfe/trunk/unittests/Tooling/ToolingTest.cpp?rev=182864&r1=182863&r > 2=182864&view=diff > ========================================================== > ==================== > --- cfe/trunk/unittests/Tooling/ToolingTest.cpp (original) > +++ cfe/trunk/unittests/Tooling/ToolingTest.cpp Wed May 29 11:01:10 > 2013 > @@ -131,20 +131,26 @@ TEST(ToolInvocation, TestMapVirtualFile) > EXPECT_TRUE(Invocation.run()); > } > > -struct VerifyEndCallback : public EndOfSourceFileCallback { > - VerifyEndCallback() : Called(0), Matched(false) {} > - virtual void run() { > - ++Called; > +struct VerifyEndCallback : public SourceFileCallbacks { > + VerifyEndCallback() : BeginCalled(0), EndCalled(0), Matched(false) {} > + virtual bool BeginSource(CompilerInstance &CI, > + StringRef Filename) LLVM_OVERRIDE { > + ++BeginCalled; > + return true; > + } > + virtual void EndSource() { > + ++EndCalled; > } > ASTConsumer *newASTConsumer() { > return new FindTopLevelDeclConsumer(&Matched); > } > - unsigned Called; > + unsigned BeginCalled; > + unsigned EndCalled; > bool Matched; > }; > > #if !defined(_WIN32) > -TEST(newFrontendActionFactory, InjectsEndOfSourceFileCallback) { > +TEST(newFrontendActionFactory, InjectsSourceFileCallbacks) { > VerifyEndCallback EndCallback; > > FixedCompilationDatabase Compilations("/", > std::vector<std::string>()); > @@ -159,7 +165,8 @@ TEST(newFrontendActionFactory, InjectsEn > Tool.run(newFrontendActionFactory(&EndCallback, &EndCallback)); > > EXPECT_TRUE(EndCallback.Matched); > - EXPECT_EQ(2u, EndCallback.Called); > + EXPECT_EQ(2u, EndCallback.BeginCalled); > + EXPECT_EQ(2u, EndCallback.EndCalled); > } > #endif > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
