[PATCH] D61643: [PragmaHandler][NFC] Expose `#pragma` location

Joel E. Denny via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 21 07:30:48 PDT 2019


jdenny marked an inline comment as done.
jdenny added inline comments.


================
Comment at: clang/docs/ClangPlugins.rst:58
     ExamplePragmaHandler() : PragmaHandler("example_pragma") { }
-    void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+    void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
                       Token &PragmaTok) {
----------------
jdenny wrote:
> lebedev.ri wrote:
> > jdenny wrote:
> > > lebedev.ri wrote:
> > > > Hmm.
> > > > This will have effects on out-of-tree plugins that define pragmas.
> > > > I'm not sure how to proceed here, just notify cfe-dev and move on?
> > > We could do something like this in `PragmaHandler`:
> > > 
> > > ```
> > >   virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
> > >                             Token &FirstToken) {
> > >     llvm_unreachable("deprecated HandlePragma unexpectedly called");
> > >   }
> > >   virtual void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
> > >                             Token &FirstToken) {
> > >     HandlePragma(PP, Introducer.Kind, FirstToken);
> > >   }
> > > ```
> > > 
> > > The derived class could override either one.  Unfortunately, if it didn't override either, the error is then at run time instead of compile time as it is now.
> > > 
> > > Whether this is worth it, I don't know.  Who's the right person to answer this question?
> > I think mailing cfe-dev will get this question the most visibility.
> > Though i //think// the solution will be to go with the current patch.
> I tried that at
> 
> http://lists.llvm.org/pipermail/cfe-dev/2019-May/062297.htm
> 
> but it's been a week without a response.  I'm happy to try again, perhaps with a shorter more general email about `PragmaHandler` backward compatibility that might catch a different set of eyes.  Is it worth it, or should we just assume no response so far means no one thinks it's an issue?  Thanks.
Sorry, copy and paste error.  The URL is:

http://lists.llvm.org/pipermail/cfe-dev/2019-May/062297.html


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61643/new/

https://reviews.llvm.org/D61643





More information about the cfe-commits mailing list