[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