[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location
Joel E. Denny via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon May 6 07:56:13 PDT 2019
jdenny added a comment.
In D61509#1491898 <https://reviews.llvm.org/D61509#1491898>, @ABataev wrote:
> Again, I don't see a single point why would we need this. I don't think there is a big difference between the location of pragma keyword and `omp` keyword.
@lebedev.ri : You said you're not sure changing the current start location would matter for diagnostics, but do you believe we need to be concerned about backward compatibility here for `Rewriter` users?
> Annotation tokens already have extra locations. Do you need the third one?
Some pragmas already use that extra location. Roman suggested consistency among all pragmas, and it would be nice if they stored the `#pragma` location in a consistent manner.
> Plus, how you're going to store this location in the AST? We don't need any extra base classes. If you want to store this in AST nodes, add a new field to the existing classes and extended constructors/Create/(de-)serialization functions.
If we're just adjusting OpenMP, I agree. However, Roman suggested consistency among all pragmas, and I thought the base clase would make that easier.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61509/new/
https://reviews.llvm.org/D61509
More information about the cfe-commits
mailing list