[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