On Mon, Jun 3, 2013 at 5:06 PM, Vane, Edwin <[email protected]> wrote:
> 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?" > Well, "why is it missing" is easy to answer: the minimal needed solution to run a clang tool is to hook into the end callback. If the callback is a problem I'll remove it and add a custom FrontendAction > where it's needed in the migrator. I don't think it's harmful (as it's usually not in the "visible" interface space anyway). One question is whether we need the CompilerInstance in the callback, as that is a kind of coupling that goes beyond interface symmetry ... Thoughts? /Manuel > > > -----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
