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

Joel E. Denny via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 22 07:58:21 PDT 2019


jdenny added a subscriber: Meinersbur.
jdenny added a comment.

Now that D61643 <https://reviews.llvm.org/D61643> is pushed, we're back to this patch.  My recollection is there are two remaining issues:

1. Should we store both the `#pragma` location and the `omp` location in the AST, or is it fine to just replace the latter location with the former?  If we choose to store both, we still haven't settled on an implementation, which likely will involve non-trivial changes in the parser and the AST.  For OpenMP, @ABataev said replacing should be fine.

2. Should we adjust all non-OpenMP pragmas in the same way immediately, or should we adjust them gradually as the need arises?

If the answers to the above questions are "replace" and "gradually", then I believe this patch is ready.  (It's just a tweak in `clang/lib/Parse/ParsePragma.cpp` and a ton of test updates.)

By the way, in D61643 <https://reviews.llvm.org/D61643>, @Meinersbur pointed out that @rsmith also wanted to see diagnostics point at a `#pragma`:  https://bugs.llvm.org/show_bug.cgi?id=41514#c1

If @rsmith and others don't get a chance to chime in, then I suppose an RFC should be next.


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

https://reviews.llvm.org/D61509





More information about the cfe-commits mailing list