r182864 - Tooling: Call back for both begin and end of sources
Manuel Klimek
klimek at google.com
Mon Jun 3 08:16:53 PDT 2013
On Mon, Jun 3, 2013 at 5:06 PM, Vane, Edwin <edwin.vane at intel.com> 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:klimek at google.com]
> > Sent: Monday, June 03, 2013 8:59 AM
> > To: Vane, Edwin
> > Cc: cfe-commits at cs.uiuc.edu
> > 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 <edwin.vane at intel.com>
> 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
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130603/1c450569/attachment.html>
More information about the cfe-commits
mailing list