[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. Thanks for everyone's responses so far. At this point, I'm going to follow the uncontroversial suggestions from @lebedev.ri: create a `struct PragmaIntroducer`, and split the OpenMP work into a second patch. That will hopefully make it easier for @rsmith or others to

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

ABataev added a comment. In D61509#1491979 , @jdenny wrote: > In 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

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In 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

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

ABataev added a comment. In D61509#1491397 , @jdenny wrote: > In D61509#1491209 , @ABataev wrote: > > > In D61509#1491204 , @lebedev.ri > > wrote: > > > > > In

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1491537 , @lebedev.ri wrote: > In D61509#1491397 , @jdenny wrote: > > > In D61509#1491209 , @ABataev wrote: > > > > > In D61509#1491204

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

lebedev.ri added a comment. In D61509#1491397 , @jdenny wrote: > In D61509#1491209 , @ABataev wrote: > > > In D61509#1491204 , @lebedev.ri > > wrote: > > > > > In

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1491209 , @ABataev wrote: > In D61509#1491204 , @lebedev.ri > wrote: > > > In D61509#1491158 , @jdenny wrote: > > > > > In

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

ABataev added a comment. In D61509#1491204 , @lebedev.ri wrote: > In D61509#1491158 , @jdenny wrote: > > > In D61509#1491119 , @lebedev.ri > > wrote: > > > > > I recommend

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

lebedev.ri added a comment. In D61509#1491158 , @jdenny wrote: > In D61509#1491119 , @lebedev.ri > wrote: > > > I recommend to split this into two parts - changing `PragmaIntroducerKind > > Introducer` to > >

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1491119 , @lebedev.ri wrote: > I recommend to split this into two parts - changing `PragmaIntroducerKind > Introducer` to > `struct NameMe { PragmaIntroducerKind Kind; SourceLocation Loc};`, and the > actual openmp

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

lebedev.ri added reviewers: aaron.ballman, lebedev.ri. lebedev.ri added a comment. I recommend to split this into two parts - changing `PragmaIntroducerKind Introducer` to `struct NameMe { PragmaIntroducerKind Kind; SourceLocation Loc};`, and the actual openmp change. For that change, just

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

ABataev added a comment. The OpenMP part looks good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61509/new/ https://reviews.llvm.org/D61509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny updated this revision to Diff 198046. jdenny edited the summary of this revision. jdenny added a comment. Herald added a subscriber: jfb. As discussed, implement OpenMP solution #1 (one-line change in `PragmaOpenMPHandler::HandlePragma` in `ParsePragma.cpp`), and update tests. CHANGES

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. Thanks for explaining. I'll proceed with solution #1. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61509/new/ https://reviews.llvm.org/D61509 ___ cfe-commits mailing list

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

ABataev added a comment. In D61509#1489845 , @jdenny wrote: > In D61509#1489771 , @ABataev wrote: > > > In D61509#1489761 , @jdenny wrote: > > > > > In D61509#1489752

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1489771 , @ABataev wrote: > In D61509#1489761 , @jdenny wrote: > > > In D61509#1489752 , @ABataev wrote: > > > > > If the patch is going to

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

ABataev added a comment. In D61509#1489761 , @jdenny wrote: > In D61509#1489752 , @ABataev wrote: > > > If the patch is going to be accepted, then definitely it must be solution > > #1. > > > I certainly have no

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1489752 , @ABataev wrote: > If the patch is going to be accepted, then definitely it must be solution #1. I certainly have no objection to that and will be happy to implement it, but can you help me to understand the

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

ABataev added a comment. If the patch is going to be accepted, then definitely it must be solution #1. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61509/new/ https://reviews.llvm.org/D61509 ___

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny created this revision. jdenny added reviewers: ABataev, rnk, Eugene.Zelenko, akyrtzi, rsmith. Herald added subscribers: jdoerfert, dexonsmith, guansong. Herald added a project: clang. Currently, an OpenMP AST node's recorded location starts at the `omp` token after the `#pragma` token, and