[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

Michael Kruse via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 22 12:36:30 PDT 2019


Meinersbur added a comment.

In D61509#1512330 <https://reviews.llvm.org/D61509#1512330>, @jdenny wrote:

> In D61509#1512321 <https://reviews.llvm.org/D61509#1512321>, @Meinersbur wrote:
>
> > In D61509#1512311 <https://reviews.llvm.org/D61509#1512311>, @jdenny wrote:
> >
> > > 2. I too think it likely makes sense to adjust them all eventually.  But do people think it's important to write patches adjusting all pragmas before pushing the adjustment for any of them?
> >
> >
> > I am not sure I understand. Do you mean whether you need all patches for each pragma to be accepted before you can commit the first? This is not that case.
>
>
> @lebedev.ri expressed concern that it might not be acceptable to migrate all pragmas in the same way.  That would suggest we must handle them all before committing any.
>
> > IMHO you can even put all of it into a single patch as it should be very straightforward. The most work is adapting the tests.
>
> I would think different people would want to review different pragmas, so separate patches would be better, but I'm happy to be corrected as I haven't explored who owns what here.


AFICS it is changing ` Tok.setLocation(FirstTok.getLocation());` to ` Tok.setLocation(Introducer.Loc);` for most PragmaHandlers that emit an annotation token. To be on the safe side, you can create a patch for each PragmaHandler individually (otherwise a review may request to spit them up). You can commit each accepted patch immediately unless a reviewer mentions that you should wait for $event to happen, like @aaron.ballman just did.


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

https://reviews.llvm.org/D61509





More information about the cfe-commits mailing list