[OpenMP] Set pragma start loc to `#pragma` loc

[jdenny]

Summary

This patch adjusts PragmaOpenMPHandler to set the location of
tok::annot_pragma_openmp to the #pragma location instead of the
omp location so that the former becomes the start location of the
OpenMP AST node. This can be useful when, for example, rewriting a
directive using Clang's Rewrite facility. Most of this patch updates
tests for changes to locations in diagnostics and -ast-dump output.

[ABataev]

If the patch is going to be accepted, then definitely it must be solution #1.

[jdenny]

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 rationale? (Thanks for your quick response!)

[ABataev]

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 objection to that and will be happy to implement it, but can you help me to understand the rationale? (Thanks for your quick response!)

Another one annotation token may significantly change the parsing process. It will require a lot of rework in the parsing of OpenMP pragmas plus may lead to some unpredictable results like endless parsing in some cases etc. It is much easier to change the tests rather than modify the whole parsing procedure.

[jdenny]

In D61509#1489771, @ABataev wrote:

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 objection to that and will be happy to implement it, but can you help me to understand the rationale? (Thanks for your quick response!)

Another one annotation token may significantly change the parsing process. It will require a lot of rework in the parsing of OpenMP pragmas plus may lead to some unpredictable results like endless parsing in some cases etc.

It seems simple to just consistently replace the one token with two consecutive tokens. That much is easy enough for me to try out (without yet integrating the locations into the AST, which would take much longer). Would you expect the test suite to catch the parsing problems you anticipate?

It is much easier to change the tests rather than modify the whole parsing procedure.

I presented that argument poorly. My main concern for solution #1 is that it might break backward compatibility for existing users of the AST. However, I don't actually have evidence any user cares. Do we generally offer backward compatibility guarantees here at all? If this is not an important concern, then solution #1 seems obviously right to me too.

[ABataev]

In D61509#1489845, @jdenny wrote:

In D61509#1489771, @ABataev wrote:

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 objection to that and will be happy to implement it, but can you help me to understand the rationale? (Thanks for your quick response!)

Another one annotation token may significantly change the parsing process. It will require a lot of rework in the parsing of OpenMP pragmas plus may lead to some unpredictable results like endless parsing in some cases etc.

It seems simple to just consistently replace the one token with two consecutive tokens. That much is easy enough for me to try out (without yet integrating the locations into the AST, which would take much longer). Would you expect the test suite to catch the parsing problems you anticipate?

It is much easier to change the tests rather than modify the whole parsing procedure.

I presented that argument poorly. My main concern for solution #1 is that it might break backward compatibility for existing users of the AST. However, I don't actually have evidence any user cares. Do we generally offer backward compatibility guarantees here at all? If this is not an important concern, then solution #1 seems obviously right to me too.

I rather doubt there was many users. Plus, this change will change just the source location, nothing else. So I still suggest to use approach #1.
About test suite, we have some tests to try to catch the endless parsing case (it was reported already earlier and eas fixed), but the testing is not full, of course.
Also, the task of replacing one anotation token by 2 otehrs is not so straightforward, it might require some additional code changes and logic changes.

[jdenny]

Thanks for explaining. I'll proceed with solution #1.

[jdenny]

As discussed, implement OpenMP solution #1 (one-line change in PragmaOpenMPHandler::HandlePragma in ParsePragma.cpp), and update tests.

[ABataev]

The OpenMP part looks good.

[lebedev.ri]

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 basing off the clang-tidy diff, neither variant is ideal, so i'd like to see the proposed use-case.

clang/include/clang/Lex/Pragma.h

68–70

PragmaIntroducerKind Introducer and SourceLocation IntroducerLoc are talking about the same thing, are they not?
They should be put into some new struct, instead of adding a new parameter.

[jdenny]

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 change.

Sure, I'll work on that. What about NameMe = PragmaIntroducer?

For that change, just basing off the clang-tidy diff, neither variant is ideal,

Do you mean that it's better for diagnostics that point to a pragma not to include the #pragma in their locations? If so, why is that?

The other variant I originally proposed doesn't alter existing locations and so wouldn't affect diagnostics, but Alexey pointed out potential issues for changes it requires in the parser.

so i'd like to see the proposed use-case.

That's a bit tricky. I'll explain.

My use case right now is not for OpenMP locations but for OpenACC locations in our Clacc project, which is not upstream. Clacc is calling Rewriter and passing locations from the OpenACC AST in order to rewrite the OpenACC pragmas to OpenMP. Without the location for an OpenACC pragma's #pragma, Clacc cannot easily remove an OpenACC pragma or insert code before it. At least not that I found. Of course, I'd be happy for someone to explain to me how Rewriter was designed to be used differently. I'd also be happy to share relevant code samples from Clacc if that would help.

So what does this have to do with OpenMP locations? I'm anticipating that someone will one day want to use Rewriter in this way with other pragmas, so I'm offering some scaffolding toward that goal for all pragmas, and I'm completing the fix only for OpenMP, which is the existing pragma set I understand best. Moreover, offering this now facilitates keeping Clacc up to date with master and eventually considering it for upstreaming.

Thanks for your responses.

[lebedev.ri]

In D61509#1491158, @jdenny wrote:

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 change.

Sure, I'll work on that. What about NameMe = PragmaIntroducer?

Could work. And then move PragmaIntroducerKind into it.
I believe this part of the refactoring is completely uncontroversial.

For that change, just basing off the clang-tidy diff, neither variant is ideal,

Do you mean that it's better for diagnostics that point to a pragma not to include the #pragma in their locations? If so, why is that?

I'm not sure either one is better than the other one.

I have two concerns:

• I fear this would result in inconsistency with other pragmas, since this will only change openmp-ones. I don't know if it will be accepted to migrate the rest of them in the same way.
• This use-case requires having the location of #pragma, so the entire AST is migrating to store it. But the current location will no longer be accessible from AST.

I see two paths forward:

• Mail cfe-dev, and suggest to do this change for *all* pragmas. Either this is ok for all of them, or none of them.
• Moar abstractions - how about not changing the startloc of openmp directives, but instead add some baseclass to OMPDirective class (& every other class that is created from pragma), that would record the PragmaIntroducer?

The other variant I originally proposed doesn't alter existing locations and so wouldn't affect diagnostics, but Alexey pointed out potential issues for changes it requires in the parser.

so i'd like to see the proposed use-case.

That's a bit tricky. I'll explain.

My use case right now is not for OpenMP locations but for OpenACC locations in our Clacc project, which is not upstream. Clacc is calling Rewriter and passing locations from the OpenACC AST in order to rewrite the OpenACC pragmas to OpenMP. Without the location for an OpenACC pragma's #pragma, Clacc cannot easily remove an OpenACC pragma or insert code before it. At least not that I found. Of course, I'd be happy for someone to explain to me how Rewriter was designed to be used differently. I'd also be happy to share relevant code samples from Clacc if that would help.

So what does this have to do with OpenMP locations? I'm anticipating that someone will one day want to use Rewriter in this way with other pragmas, so I'm offering some scaffolding toward that goal for all pragmas, and I'm completing the fix only for OpenMP, which is the existing pragma set I understand best. Moreover, offering this now facilitates keeping Clacc up to date with master and eventually considering it for upstreaming.

Thanks for your responses.

[ABataev]

In D61509#1491204, @lebedev.ri wrote:

In D61509#1491158, @jdenny wrote:

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 change.

Sure, I'll work on that. What about NameMe = PragmaIntroducer?

Could work. And then move PragmaIntroducerKind into it.
I believe this part of the refactoring is completely uncontroversial.

For that change, just basing off the clang-tidy diff, neither variant is ideal,

Do you mean that it's better for diagnostics that point to a pragma not to include the #pragma in their locations? If so, why is that?

I'm not sure either one is better than the other one.

I have two concerns:

• I fear this would result in inconsistency with other pragmas, since this will only change openmp-ones. I don't know if it will be accepted to migrate the rest of them in the same way.
• This use-case requires having the location of #pragma, so the entire AST is migrating to store it. But the current location will no longer be accessible from AST.

I see two paths forward:

• Mail cfe-dev, and suggest to do this change for *all* pragmas. Either this is ok for all of them, or none of them.
• Moar abstractions - how about not changing the startloc of openmp directives, but instead add some baseclass to OMPDirective class (& every other class that is created from pragma), that would record the PragmaIntroducer?

I'm against this solution. I don't see any reasons why we should do this. Instead, we're getting a lot of pain with parsing and maintenance.

The other variant I originally proposed doesn't alter existing locations and so wouldn't affect diagnostics, but Alexey pointed out potential issues for changes it requires in the parser.

so i'd like to see the proposed use-case.

That's a bit tricky. I'll explain.

My use case right now is not for OpenMP locations but for OpenACC locations in our Clacc project, which is not upstream. Clacc is calling Rewriter and passing locations from the OpenACC AST in order to rewrite the OpenACC pragmas to OpenMP. Without the location for an OpenACC pragma's #pragma, Clacc cannot easily remove an OpenACC pragma or insert code before it. At least not that I found. Of course, I'd be happy for someone to explain to me how Rewriter was designed to be used differently. I'd also be happy to share relevant code samples from Clacc if that would help.

So what does this have to do with OpenMP locations? I'm anticipating that someone will one day want to use Rewriter in this way with other pragmas, so I'm offering some scaffolding toward that goal for all pragmas, and I'm completing the fix only for OpenMP, which is the existing pragma set I understand best. Moreover, offering this now facilitates keeping Clacc up to date with master and eventually considering it for upstreaming.

Thanks for your responses.

[jdenny]

In D61509#1491209, @ABataev wrote:

In D61509#1491204, @lebedev.ri wrote:

In D61509#1491158, @jdenny wrote:

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 change.

Sure, I'll work on that. What about NameMe = PragmaIntroducer?

Could work. And then move PragmaIntroducerKind into it.
I believe this part of the refactoring is completely uncontroversial.

For that change, just basing off the clang-tidy diff, neither variant is ideal,

Do you mean that it's better for diagnostics that point to a pragma not to include the #pragma in their locations? If so, why is that?

I'm not sure either one is better than the other one.

I have two concerns:

• I fear this would result in inconsistency with other pragmas, since this will only change openmp-ones. I don't know if it will be accepted to migrate the rest of them in the same way.
• This use-case requires having the location of #pragma, so the entire AST is migrating to store it. But the current location will no longer be accessible from AST.

I see two paths forward:

• Mail cfe-dev, and suggest to do this change for *all* pragmas. Either this is ok for all of them, or none of them.
• Moar abstractions - how about not changing the startloc of openmp directives,

My alternative proposal was exactly that. A difficulty is how to pass the #pragma location to the OpenMP AST node constructors. PragmaOpenMPHandler::HandlePragma passes locations via the tok::annot_pragma_openmp and tok::annot_pragma_openmp_end tokens, so where do we pass this new location? I proposed creating a third token, and Alexey was concerned over the parsing problems that would create.

but instead add some baseclass to `OMPDirective` class (& every other class that is created from pragma), that would record the `PragmaIntroducer`?

I agree that a base class would be a nice way to extend all pragma classes with the PragmaIntroducer.

I'm against this solution. I don't see any reasons why we should do this. Instead, we're getting a lot of pain with parsing and maintenance.

One way to avoid creating an extra token would be to widen the Token class to store the additional location. The Token documentation says it's not intended to be space efficient. How does that sound to people?

[lebedev.ri]

In D61509#1491397, @jdenny wrote:

In D61509#1491209, @ABataev wrote:

In D61509#1491204, @lebedev.ri wrote:

In D61509#1491158, @jdenny wrote:

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 change.

Sure, I'll work on that. What about NameMe = PragmaIntroducer?

Could work. And then move PragmaIntroducerKind into it.
I believe this part of the refactoring is completely uncontroversial.

For that change, just basing off the clang-tidy diff, neither variant is ideal,

Do you mean that it's better for diagnostics that point to a pragma not to include the #pragma in their locations? If so, why is that?

I'm not sure either one is better than the other one.

I have two concerns:

• I fear this would result in inconsistency with other pragmas, since this will only change openmp-ones. I don't know if it will be accepted to migrate the rest of them in the same way.
• This use-case requires having the location of #pragma, so the entire AST is migrating to store it. But the current location will no longer be accessible from AST.

I see two paths forward:

• Mail cfe-dev, and suggest to do this change for *all* pragmas. Either this is ok for all of them, or none of them.
• Moar abstractions - how about not changing the startloc of openmp directives,

My alternative proposal was exactly that. A difficulty is how to pass the #pragma location to the OpenMP AST node constructors. PragmaOpenMPHandler::HandlePragma passes locations via the tok::annot_pragma_openmp and tok::annot_pragma_openmp_end tokens, so where do we pass this new location? I proposed creating a third token, and Alexey was concerned over the parsing problems that would create.

but instead add some baseclass to `OMPDirective` class (& every other class that is created from pragma), that would record the `PragmaIntroducer`?

I agree that a base class would be a nice way to extend all pragma classes with the PragmaIntroducer.

I'm against this solution. I don't see any reasons why we should do this. Instead, we're getting a lot of pain with parsing and maintenance.

One way to avoid creating an extra token would be to widen the Token class to store the additional location. The Token documentation says it's not intended to be space efficient. How does that sound to people?

That was my proposal, yes.

clang-tools-extra/test/clang-tidy/openmp-exception-escape.cpp

1 ↗

(On Diff #198046)

Changes to this file will no longer be needed, i have changed the check.

[jdenny]

In D61509#1491537, @lebedev.ri wrote:

In D61509#1491397, @jdenny wrote:

In D61509#1491209, @ABataev wrote:

In D61509#1491204, @lebedev.ri wrote:

In D61509#1491158, @jdenny wrote:

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 change.

Sure, I'll work on that. What about NameMe = PragmaIntroducer?

Could work. And then move PragmaIntroducerKind into it.
I believe this part of the refactoring is completely uncontroversial.

For that change, just basing off the clang-tidy diff, neither variant is ideal,

Do you mean that it's better for diagnostics that point to a pragma not to include the #pragma in their locations? If so, why is that?

I'm not sure either one is better than the other one.

I have two concerns:

• I fear this would result in inconsistency with other pragmas, since this will only change openmp-ones. I don't know if it will be accepted to migrate the rest of them in the same way.
• This use-case requires having the location of #pragma, so the entire AST is migrating to store it. But the current location will no longer be accessible from AST.

I see two paths forward:

• Mail cfe-dev, and suggest to do this change for *all* pragmas. Either this is ok for all of them, or none of them.
• Moar abstractions - how about not changing the startloc of openmp directives,

My alternative proposal was exactly that. A difficulty is how to pass the #pragma location to the OpenMP AST node constructors. PragmaOpenMPHandler::HandlePragma passes locations via the tok::annot_pragma_openmp and tok::annot_pragma_openmp_end tokens, so where do we pass this new location? I proposed creating a third token, and Alexey was concerned over the parsing problems that would create.

but instead add some baseclass to `OMPDirective` class (& every other class that is created from pragma), that would record the `PragmaIntroducer`?

I agree that a base class would be a nice way to extend all pragma classes with the PragmaIntroducer.

I'm against this solution. I don't see any reasons why we should do this. Instead, we're getting a lot of pain with parsing and maintenance.

One way to avoid creating an extra token would be to widen the Token class to store the additional location. The Token documentation says it's not intended to be space efficient. How does that sound to people?

That was my proposal, yes.

@ABataev : Does that address your concerns?

[ABataev]

In D61509#1491397, @jdenny wrote:

In D61509#1491209, @ABataev wrote:

In D61509#1491204, @lebedev.ri wrote:

In D61509#1491158, @jdenny wrote:

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 change.

Sure, I'll work on that. What about NameMe = PragmaIntroducer?

Could work. And then move PragmaIntroducerKind into it.
I believe this part of the refactoring is completely uncontroversial.

For that change, just basing off the clang-tidy diff, neither variant is ideal,

Do you mean that it's better for diagnostics that point to a pragma not to include the #pragma in their locations? If so, why is that?

I'm not sure either one is better than the other one.

I have two concerns:

• I fear this would result in inconsistency with other pragmas, since this will only change openmp-ones. I don't know if it will be accepted to migrate the rest of them in the same way.
• This use-case requires having the location of #pragma, so the entire AST is migrating to store it. But the current location will no longer be accessible from AST.

I see two paths forward:

• Mail cfe-dev, and suggest to do this change for *all* pragmas. Either this is ok for all of them, or none of them.
• Moar abstractions - how about not changing the startloc of openmp directives,

My alternative proposal was exactly that. A difficulty is how to pass the #pragma location to the OpenMP AST node constructors. PragmaOpenMPHandler::HandlePragma passes locations via the tok::annot_pragma_openmp and tok::annot_pragma_openmp_end tokens, so where do we pass this new location? I proposed creating a third token, and Alexey was concerned over the parsing problems that would create.

but instead add some baseclass to `OMPDirective` class (& every other class that is created from pragma), that would record the `PragmaIntroducer`?

I agree that a base class would be a nice way to extend all pragma classes with the PragmaIntroducer.

I'm against this solution. I don't see any reasons why we should do this. Instead, we're getting a lot of pain with parsing and maintenance.

One way to avoid creating an extra token would be to widen the Token class to store the additional location. The Token documentation says it's not intended to be space efficient. How does that sound to people?

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.

In D61509#1491882, @jdenny wrote:

In D61509#1491537, @lebedev.ri wrote:

In D61509#1491397, @jdenny wrote:

In D61509#1491209, @ABataev wrote:

In D61509#1491204, @lebedev.ri wrote:

In D61509#1491158, @jdenny wrote:

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 change.

Sure, I'll work on that. What about NameMe = PragmaIntroducer?

Could work. And then move PragmaIntroducerKind into it.
I believe this part of the refactoring is completely uncontroversial.

For that change, just basing off the clang-tidy diff, neither variant is ideal,

Do you mean that it's better for diagnostics that point to a pragma not to include the #pragma in their locations? If so, why is that?

I'm not sure either one is better than the other one.

I have two concerns:

• I fear this would result in inconsistency with other pragmas, since this will only change openmp-ones. I don't know if it will be accepted to migrate the rest of them in the same way.
• This use-case requires having the location of #pragma, so the entire AST is migrating to store it. But the current location will no longer be accessible from AST.

I see two paths forward:

• Mail cfe-dev, and suggest to do this change for *all* pragmas. Either this is ok for all of them, or none of them.
• Moar abstractions - how about not changing the startloc of openmp directives,

My alternative proposal was exactly that. A difficulty is how to pass the #pragma location to the OpenMP AST node constructors. PragmaOpenMPHandler::HandlePragma passes locations via the tok::annot_pragma_openmp and tok::annot_pragma_openmp_end tokens, so where do we pass this new location? I proposed creating a third token, and Alexey was concerned over the parsing problems that would create.

but instead add some baseclass to `OMPDirective` class (& every other class that is created from pragma), that would record the `PragmaIntroducer`?

I agree that a base class would be a nice way to extend all pragma classes with the PragmaIntroducer.

I'm against this solution. I don't see any reasons why we should do this. Instead, we're getting a lot of pain with parsing and maintenance.

One way to avoid creating an extra token would be to widen the Token class to store the additional location. The Token documentation says it's not intended to be space efficient. How does that sound to people?

That was my proposal, yes.

@ABataev : Does that address your concerns?

Annotation tokens already have extra locations. Do you need the third one? 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.

[jdenny]

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 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.

[ABataev]

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 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?

For diagnostics, we don't use the location of #pragma or omp keywords often. If the diagnostic must be emitted for the whole pragma, it would be better to emit it for #pragma keyword, not omp keyword. I don't think there any rewriter users for OpenMP. Plus, if the problem exist (we point to omp keyword rather tan #pragma) better to fix this, if you can.

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.

That was just a suggestion. YOu'd better to dicuss this with @rsmith.

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.

Handling of other pragmas is completely different story. From OpenMP implementation point of view, we'll ned to store this extra location somewhere in the AST nodes for OpenMP. And you will need to do this for all the OpenMP pragmas (executable and declarative). If you're ging to store this location in AST nodes for OpenMP pragmas, I suggest not to add the new base class, but just extend the existing classes with the new data member for the location of the #pragma keyword.

[jdenny]

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 consider what we have now and what should be done for other pragmas now, if anything.

[jdenny]

As suggested, I've created a struct PragmaIntroducer and I've reduced this patch not to include the OpenMP changes. I'll add a new patch with the OpenMP changes soon.

There are a few additional changes here reviewers might want to check out:

• This time I noticed that clang/docs/ClangPlugins.rst and clang/examples/AnnotateFunctions need to be updated for the change to PragmaHandler parameters, so I did that. Does that represent an important break in backward compatibility?

• I've also used PragmaIntroducer to simplify clang::Preprocessor::HandlePragmaDirective, which already accepted a SourceLocation and PragmaIntroducerKind before any of my patches.

• I didn't do the same for clang::PPCallbacks::PragmaDirective because that changes the API documented in clang-tools-extra/docs/pp-trace.rst, and I'm not sure about backward compatibility guarantees there.

[lebedev.ri]

It would have been better to submit this refactor as a new patch..

[jdenny]

In D61509#1493703, @lebedev.ri wrote:

It would have been better to submit this refactor as a new patch..

Sorry, I didn't realize that was the norm. I can do that now if it would help. I can also revert changes to this patch if the goal is to make it easier to reference the old version.

[lebedev.ri]

In D61509#1493714, @jdenny wrote:

In D61509#1493703, @lebedev.ri wrote:

It would have been better to submit this refactor as a new patch..

Sorry, I didn't realize that was the norm. I can do that now if it would help. I can also revert changes to this patch if the goal is to make it easier to reference the old version.

I think that would be good. This current diff would be a simple NFC cleanup, i think i will signoff it even.

[jdenny]

In D61509#1493718, @lebedev.ri wrote:

In D61509#1493714, @jdenny wrote:

In D61509#1493703, @lebedev.ri wrote:

It would have been better to submit this refactor as a new patch..

Sorry, I didn't realize that was the norm. I can do that now if it would help. I can also revert changes to this patch if the goal is to make it easier to reference the old version.

I think that would be good. This current diff would be a simple NFC cleanup, i think i will signoff it even.

Thanks, I'll do that.

For future reference, I assume abandoning and starting a new review is better here because I'm splitting the patch. Is there a more general rule of thumb on this?

[lebedev.ri]

I don't see why this differential can't be updated to only contain the remaining part
of the diff (for the actual OpenMP change), after splitting the NFC refactoring part.

[jdenny]

In D61509#1493729, @lebedev.ri wrote:

I don't see why this differential can't be updated to only contain the remaining part
of the diff (for the actual OpenMP change), after splitting the NFC refactoring part.

OK

[jdenny]

As requested, replace this patch with just the OpenMP changes.

[jdenny]

Now that 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, @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.

[Meinersbur]

1. Is there a diagnostic that would point to the omp token? As much as I like complete info (such as SourceLoc of semicolons), I cannot think of a use case for it.
2. I would like to see all of them all adjusted. There is an immediate improvement in that improves the diagnostic output.

[ABataev]

In D61509#1512293, @Meinersbur wrote:

1. Is there a diagnostic that would point to the omp token? As much as I like complete info (such as SourceLoc of semicolons), I cannot think of a use case for it.
2. I would like to see all of them all adjusted. There is an immediate improvement in that improves the diagnostic output.

1. Nope, there is no omp specific diagnostics

[jdenny]

In D61509#1512294, @ABataev wrote:

In D61509#1512293, @Meinersbur wrote:

1. Is there a diagnostic that would point to the omp token? As much as I like complete info (such as SourceLoc of semicolons), I cannot think of a use case for it.
2. I would like to see all of them all adjusted. There is an immediate improvement in that improves the diagnostic output.

1. Nope, there is no omp specific diagnostics

1. Based on the test suite changes this patch makes, I agree for OpenMP. I haven't explored other pragmas.

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?

[Meinersbur]

In 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. 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.

[jdenny]

In D61509#1512321, @Meinersbur wrote:

In 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.

[aaron.ballman]

In D61509#1512090, @jdenny wrote:

Now that 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.

Storing both would be nice, but not required, IMO. Storing the location of the pragma "namespace" would be useful for third-party tooling purposes, but I don't think we have a need for it in the frontend otherwise.

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

We usually do incremental improvement unless that's impossible or would leave things in a badly inconsistent state. I don't think we need to adjust everything immediately in this case, but we should strive for quickly reaching eventual consistency.

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, @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.

[jdenny]

In D61509#1512488, @aaron.ballman wrote:

In D61509#1512090, @jdenny wrote:

Now that 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.

Storing both would be nice, but not required, IMO. Storing the location of the pragma "namespace" would be useful for third-party tooling purposes, but I don't think we have a need for it in the frontend otherwise.

Makes sense. It sounds like we should go with the #pragma location for now and add the namespace location later if someone expresses a specific need.

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

We usually do incremental improvement unless that's impossible or would leave things in a badly inconsistent state. I don't think we need to adjust everything immediately in this case, but we should strive for quickly reaching eventual consistency.

The overall vote seems to be that this patch is ready to push, and we/I should work on other pragmas soon after. I'm also happy to wait for more comments if people want more time.

[aaron.ballman]

In D61509#1512556, @jdenny wrote:

The overall vote seems to be that this patch is ready to push, and we/I should work on other pragmas soon after. I'm also happy to wait for more comments if people want more time.

These changes LGTM! I'd say give it a few days in case other reviewers would like to chime in.

[jdenny]

In D61509#1512570, @aaron.ballman wrote:

These changes LGTM!

Thanks, and thanks to everyone who chimed in.

I'd say give it a few days in case other reviewers would like to chime in.

Sure. Maybe this weekend or so.

[Meinersbur]

In D61509#1512330, @jdenny wrote:

In D61509#1512321, @Meinersbur wrote:

In 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.

[jdenny]

In D61509#1512601, @Meinersbur wrote:

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.

I think the concern was more about the effect of the replacement than how to implement it. For example, the test changes in this patch provide evidence the effect on OpenMP seems fine. There might also be external use cases, so reviewers who are more familiar with specific pragmas (like Alexey for OpenMP) might have more info on those.

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.

Sure. I discussed the possibility of adjusting all pragmas (to see the effect) before pushing any changes for exactly that reason: a reviewer previously suggested waiting due to his concerns over creating inconsistencies among pragmas, some of which might have use cases OpenMP doesn't, if we pushed the OpenMP changes in this patch immediately. I think the majority now feel it's fine to go ahead, but hopefully that isn't just because I didn't explain the previous concerns clearly. Anyway, I'll wait a few days to see if there are more comments.

clang/include/clang/Lex/Pragma.h

 //===- Pragma.h - Pragma registration and handling --------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the PragmaHandler and PragmaTable interfaces.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LEX_PRAGMA_H
 #define LLVM_CLANG_LEX_PRAGMA_H
 
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include <string>
 
 namespace clang {
 
 class PragmaNamespace;
 class Preprocessor;
class PragmaHandler {
   std::string Name;
 
 public:
   PragmaHandler() = default;
   explicit PragmaHandler(StringRef name) : Name(name) {}
   virtual ~PragmaHandler();
 
   StringRef getName() const { return Name; }
   virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+                            SourceLocation IntroducerLoc,
                             Token &FirstToken) = 0;
 
   /// getIfNamespace - If this is a namespace, return it.  This is equivalent to
   /// using a dynamic_cast, but doesn't require RTTI.
   virtual PragmaNamespace *getIfNamespace() { return nullptr; }
 };
 
 /// EmptyPragmaHandler - A pragma handler which takes no action, which can be
 /// used to ignore particular pragmas.
 class EmptyPragmaHandler : public PragmaHandler {
 public:
   explicit EmptyPragmaHandler(StringRef Name = StringRef());
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 /// PragmaNamespace - This PragmaHandler subdivides the namespace of pragmas,
 /// allowing hierarchical pragmas to be defined.  Common examples of namespaces
 /// are "\#pragma GCC", "\#pragma STDC", and "\#pragma omp", but any namespaces
 /// may be (potentially recursively) defined.
 class PragmaNamespace : public PragmaHandler {
   /// Handlers - This is a map of the handlers in this namespace with their name
public:
 
   /// RemovePragmaHandler - Remove the given handler from the
   /// namespace.
   void RemovePragmaHandler(PragmaHandler *Handler);
 
   bool IsEmpty() const { return Handlers.empty(); }
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override;
+                    SourceLocation IntroducerLoc, Token &Tok) override;
 
   PragmaNamespace *getIfNamespace() override { return this; }
 };
 
 } // namespace clang
 
 #endif // LLVM_CLANG_LEX_PRAGMA_H

clang/lib/Frontend/PrintPreprocessedOutput.cpp

struct UnknownPragmaHandler : public PragmaHandler {
   // Set to true if tokens should be expanded
   bool ShouldExpandTokens;
 
   UnknownPragmaHandler(const char *prefix, PrintPPOutputPPCallbacks *callbacks,
                        bool RequireTokenExpansion)
       : Prefix(prefix), Callbacks(callbacks),
         ShouldExpandTokens(RequireTokenExpansion) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &PragmaTok) override {
+                    SourceLocation IntroducerLoc, Token &PragmaTok) override {
     // Figure out what line we went to and insert the appropriate number of
     // newline characters.
     Callbacks->startNewLineIfNeeded();
     Callbacks->MoveToLine(PragmaTok.getLocation());
     Callbacks->OS.write(Prefix, strlen(Prefix));
 
     if (ShouldExpandTokens) {
       // The first token does not have expanded macros. Expand them, if

clang/lib/Lex/Pragma.cpp

 //===----------------------------------------------------------------------===//
 // EmptyPragmaHandler Implementation.
 //===----------------------------------------------------------------------===//
 
 EmptyPragmaHandler::EmptyPragmaHandler(StringRef Name) : PragmaHandler(Name) {}
 
 void EmptyPragmaHandler::HandlePragma(Preprocessor &PP,
                                       PragmaIntroducerKind Introducer,
+                                      SourceLocation IntroducerLoc,
                                       Token &FirstToken) {}
 
 //===----------------------------------------------------------------------===//
 // PragmaNamespace Implementation.
 //===----------------------------------------------------------------------===//
 
 PragmaNamespace::~PragmaNamespace() {
   llvm::DeleteContainerSeconds(Handlers);

 void PragmaNamespace::RemovePragmaHandler(PragmaHandler *Handler) {
   assert(Handlers.lookup(Handler->getName()) &&
          "Handler not registered in this namespace");
   Handlers.erase(Handler->getName());
 }
 
 void PragmaNamespace::HandlePragma(Preprocessor &PP,
                                    PragmaIntroducerKind Introducer,
-                                   Token &Tok) {
+                                   SourceLocation IntroducerLoc, Token &Tok) {
   // Read the 'namespace' that the directive is in, e.g. STDC.  Do not macro
   // expand it, the user can have a STDC #define, that should not affect this.
   PP.LexUnexpandedToken(Tok);
 
   // Get the handler for this token.  If there is no handler, ignore the pragma.
   PragmaHandler *Handler
     = FindHandler(Tok.getIdentifierInfo() ? Tok.getIdentifierInfo()->getName()
                                           : StringRef(),
                   /*IgnoreNull=*/false);
   if (!Handler) {
     PP.Diag(Tok, diag::warn_pragma_ignored);
     return;
   }
 
   // Otherwise, pass it down.
-  Handler->HandlePragma(PP, Introducer, Tok);
+  Handler->HandlePragma(PP, Introducer, IntroducerLoc, Tok);
 }
 
 //===----------------------------------------------------------------------===//
 // Preprocessor Pragma Directive Handling.
 //===----------------------------------------------------------------------===//
 
 /// HandlePragmaDirective - The "\#pragma" directive has been parsed.  Lex the
 /// rest of the pragma, passing it to the registered pragma handlers.
 void Preprocessor::HandlePragmaDirective(SourceLocation IntroducerLoc,
                                          PragmaIntroducerKind Introducer) {
   if (Callbacks)
     Callbacks->PragmaDirective(IntroducerLoc, Introducer);
 
   if (!PragmasEnabled)
     return;
 
   ++NumPragma;
 
   // Invoke the first level of pragma handlers which reads the namespace id.
   Token Tok;
-  PragmaHandlers->HandlePragma(*this, Introducer, Tok);
+  PragmaHandlers->HandlePragma(*this, Introducer, IntroducerLoc, Tok);
 
   // If the pragma handler didn't read the rest of the line, consume it now.
   if ((CurTokenLexer && CurTokenLexer->isParsingPreprocessorDirective())
    || (CurPPLexer && CurPPLexer->ParsingPreprocessorDirective))
     DiscardUntilEndOfDirective();
 }
 
 /// Handle_Pragma - Read a _Pragma directive, slice it up, process it, then

 
 namespace {
 
 /// PragmaOnceHandler - "\#pragma once" marks the file as atomically included.
 struct PragmaOnceHandler : public PragmaHandler {
   PragmaOnceHandler() : PragmaHandler("once") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &OnceTok) override {
+                    SourceLocation IntroducerLoc, Token &OnceTok) override {
     PP.CheckEndOfDirective("pragma once");
     PP.HandlePragmaOnce(OnceTok);
   }
 };
 
 /// PragmaMarkHandler - "\#pragma mark ..." is ignored by the compiler, and the
 /// rest of the line is not lexed.
 struct PragmaMarkHandler : public PragmaHandler {
   PragmaMarkHandler() : PragmaHandler("mark") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &MarkTok) override {
+                    SourceLocation IntroducerLoc, Token &MarkTok) override {
     PP.HandlePragmaMark();
   }
 };
 
 /// PragmaPoisonHandler - "\#pragma poison x" marks x as not usable.
 struct PragmaPoisonHandler : public PragmaHandler {
   PragmaPoisonHandler() : PragmaHandler("poison") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &PoisonTok) override {
+                    SourceLocation IntroducerLoc, Token &PoisonTok) override {
     PP.HandlePragmaPoison();
   }
 };
 
 /// PragmaSystemHeaderHandler - "\#pragma system_header" marks the current file
 /// as a system header, which silences warnings in it.
 struct PragmaSystemHeaderHandler : public PragmaHandler {
   PragmaSystemHeaderHandler() : PragmaHandler("system_header") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &SHToken) override {
+                    SourceLocation IntroducerLoc, Token &SHToken) override {
     PP.HandlePragmaSystemHeader(SHToken);
     PP.CheckEndOfDirective("pragma");
   }
 };
 
 struct PragmaDependencyHandler : public PragmaHandler {
   PragmaDependencyHandler() : PragmaHandler("dependency") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &DepToken) override {
+                    SourceLocation IntroducerLoc, Token &DepToken) override {
     PP.HandlePragmaDependency(DepToken);
   }
 };
 
 struct PragmaDebugHandler : public PragmaHandler {
   PragmaDebugHandler() : PragmaHandler("__debug") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &DebugToken) override {
+                    SourceLocation IntroducerLoc, Token &DebugToken) override {
     Token Tok;
     PP.LexUnexpandedToken(Tok);
     if (Tok.isNot(tok::identifier)) {
       PP.Diag(Tok, diag::warn_pragma_diagnostic_invalid);
       return;
     }
     IdentifierInfo *II = Tok.getIdentifierInfo();
 

 private:
   const char *Namespace;
 
 public:
   explicit PragmaDiagnosticHandler(const char *NS)
       : PragmaHandler("diagnostic"), Namespace(NS) {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &DiagToken) override {
+                    SourceLocation IntroducerLoc, Token &DiagToken) override {
     SourceLocation DiagLoc = DiagToken.getLocation();
     Token Tok;
     PP.LexUnexpandedToken(Tok);
     if (Tok.isNot(tok::identifier)) {
       PP.Diag(Tok, diag::warn_pragma_diagnostic_invalid);
       return;
     }
     IdentifierInfo *II = Tok.getIdentifierInfo();
else if (Callbacks)
       Callbacks->PragmaDiagnostic(DiagLoc, Namespace, SV, WarningName);
   }
 };
 
 /// "\#pragma hdrstop [<header-name-string>]"
 struct PragmaHdrstopHandler : public PragmaHandler {
   PragmaHdrstopHandler() : PragmaHandler("hdrstop") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &DepToken) override {
+                    SourceLocation IntroducerLoc, Token &DepToken) override {
     PP.HandlePragmaHdrstop(DepToken);
   }
 };
 
 /// "\#pragma warning(...)".  MSVC's diagnostics do not map cleanly to clang's
 /// diagnostics, so we don't really implement this pragma.  We parse it and
 /// ignore it to avoid -Wunknown-pragma warnings.
 struct PragmaWarningHandler : public PragmaHandler {
   PragmaWarningHandler() : PragmaHandler("warning") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     // Parse things like:
     // warning(push, 1)
     // warning(pop)
     // warning(disable : 1 2 3 ; error : 4 5 6 ; suppress : 7 8 9)
     SourceLocation DiagLoc = Tok.getLocation();
     PPCallbacks *Callbacks = PP.getPPCallbacks();
 
     PP.Lex(Tok);

 
 /// "\#pragma execution_character_set(...)". MSVC supports this pragma only
 /// for "UTF-8". We parse it and ignore it if UTF-8 is provided and warn
 /// otherwise to avoid -Wunknown-pragma warnings.
 struct PragmaExecCharsetHandler : public PragmaHandler {
   PragmaExecCharsetHandler() : PragmaHandler("execution_character_set") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     // Parse things like:
     // execution_character_set(push, "UTF-8")
     // execution_character_set(pop)
     SourceLocation DiagLoc = Tok.getLocation();
     PPCallbacks *Callbacks = PP.getPPCallbacks();
 
     PP.Lex(Tok);
     if (Tok.isNot(tok::l_paren)) {
struct PragmaExecCharsetHandler : public PragmaHandler {
   }
 };
 
 /// PragmaIncludeAliasHandler - "\#pragma include_alias("...")".
 struct PragmaIncludeAliasHandler : public PragmaHandler {
   PragmaIncludeAliasHandler() : PragmaHandler("include_alias") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+                    SourceLocation IntroducerLoc,
                     Token &IncludeAliasTok) override {
     PP.HandlePragmaIncludeAlias(IncludeAliasTok);
   }
 };
 
 /// PragmaMessageHandler - Handle the microsoft and gcc \#pragma message
 /// extension.  The syntax is:
 /// \code

 
 public:
   PragmaMessageHandler(PPCallbacks::PragmaMessageKind Kind,
                        StringRef Namespace = StringRef())
       : PragmaHandler(PragmaKind(Kind, true)), Kind(Kind),
         Namespace(Namespace) {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     SourceLocation MessageLoc = Tok.getLocation();
     PP.Lex(Tok);
     bool ExpectClosingParen = false;
     switch (Tok.getKind()) {
     case tok::l_paren:
       // We have a MSVC style pragma message.
       ExpectClosingParen = true;
       // Read the string.

 /// Handle the clang \#pragma module import extension. The syntax is:
 /// \code
 ///   #pragma clang module import some.module.name
 /// \endcode
 struct PragmaModuleImportHandler : public PragmaHandler {
   PragmaModuleImportHandler() : PragmaHandler("import") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     SourceLocation ImportLoc = Tok.getLocation();
 
     // Read the module name.
     llvm::SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 8>
         ModuleName;
     if (LexModuleName(PP, Tok, ModuleName))
       return;
 

 ///   #pragma clang module begin some.module.name
 ///   ...
 ///   #pragma clang module end
 /// \endcode
 struct PragmaModuleBeginHandler : public PragmaHandler {
   PragmaModuleBeginHandler() : PragmaHandler("begin") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     SourceLocation BeginLoc = Tok.getLocation();
 
     // Read the module name.
     llvm::SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 8>
         ModuleName;
     if (LexModuleName(PP, Tok, ModuleName))
       return;
 
struct PragmaModuleBeginHandler : public PragmaHandler {
   }
 };
 
 /// Handle the clang \#pragma module end extension.
 struct PragmaModuleEndHandler : public PragmaHandler {
   PragmaModuleEndHandler() : PragmaHandler("end") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     SourceLocation Loc = Tok.getLocation();
 
     PP.LexUnexpandedToken(Tok);
     if (Tok.isNot(tok::eod))
       PP.Diag(Tok, diag::ext_pp_extra_tokens_at_eol) << "pragma";
 
     Module *M = PP.LeaveSubmodule(/*ForPragma*/true);
     if (M)
       PP.EnterAnnotationToken(SourceRange(Loc), tok::annot_module_end, M);
     else
       PP.Diag(Loc, diag::err_pp_module_end_without_module_begin);
   }
 };
 
 /// Handle the clang \#pragma module build extension.
 struct PragmaModuleBuildHandler : public PragmaHandler {
   PragmaModuleBuildHandler() : PragmaHandler("build") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     PP.HandlePragmaModuleBuild(Tok);
   }
 };
 
 /// Handle the clang \#pragma module load extension.
 struct PragmaModuleLoadHandler : public PragmaHandler {
   PragmaModuleLoadHandler() : PragmaHandler("load") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     SourceLocation Loc = Tok.getLocation();
 
     // Read the module name.
     llvm::SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 8>
         ModuleName;
     if (LexModuleName(PP, Tok, ModuleName))
       return;
 
     if (Tok.isNot(tok::eod))
       PP.Diag(Tok, diag::ext_pp_extra_tokens_at_eol) << "pragma";
 
     // Load the module, don't make it visible.
     PP.getModuleLoader().loadModule(Loc, ModuleName, Module::Hidden,
                                     /*IsIncludeDirective=*/false);
   }
 };
 
 /// PragmaPushMacroHandler - "\#pragma push_macro" saves the value of the
 /// macro on the top of the stack.
 struct PragmaPushMacroHandler : public PragmaHandler {
   PragmaPushMacroHandler() : PragmaHandler("push_macro") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+                    SourceLocation IntroducerLoc,
                     Token &PushMacroTok) override {
     PP.HandlePragmaPushMacro(PushMacroTok);
   }
 };
 
 /// PragmaPopMacroHandler - "\#pragma pop_macro" sets the value of the
 /// macro to the value on the top of the stack.
 struct PragmaPopMacroHandler : public PragmaHandler {
   PragmaPopMacroHandler() : PragmaHandler("pop_macro") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+                    SourceLocation IntroducerLoc,
                     Token &PopMacroTok) override {
     PP.HandlePragmaPopMacro(PopMacroTok);
   }
 };
 
 /// PragmaARCCFCodeAuditedHandler -
 ///   \#pragma clang arc_cf_code_audited begin/end
 struct PragmaARCCFCodeAuditedHandler : public PragmaHandler {
   PragmaARCCFCodeAuditedHandler() : PragmaHandler("arc_cf_code_audited") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &NameTok) override {
+                    SourceLocation IntroducerLoc, Token &NameTok) override {
     SourceLocation Loc = NameTok.getLocation();
     bool IsBegin;
 
     Token Tok;
 
     // Lex the 'begin' or 'end'.
     PP.LexUnexpandedToken(Tok);
     const IdentifierInfo *BeginEnd = Tok.getIdentifierInfo();

 };
 
 /// PragmaAssumeNonNullHandler -
 ///   \#pragma clang assume_nonnull begin/end
 struct PragmaAssumeNonNullHandler : public PragmaHandler {
   PragmaAssumeNonNullHandler() : PragmaHandler("assume_nonnull") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &NameTok) override {
+                    SourceLocation IntroducerLoc, Token &NameTok) override {
     SourceLocation Loc = NameTok.getLocation();
     bool IsBegin;
 
     Token Tok;
 
     // Lex the 'begin' or 'end'.
     PP.LexUnexpandedToken(Tok);
     const IdentifierInfo *BeginEnd = Tok.getIdentifierInfo();

 ///
 /// \note This is
 /// <a href="http://msdn.microsoft.com/en-us/library/b6xkz944(v=vs.80).aspx">editor-only</a>
 /// pragma, just skipped by compiler.
 struct PragmaRegionHandler : public PragmaHandler {
   PragmaRegionHandler(const char *pragma) : PragmaHandler(pragma) {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &NameTok) override {
+                    SourceLocation IntroducerLoc, Token &NameTok) override {
     // #pragma region: endregion matches can be verified
     // __pragma(region): no sense, but ignored by msvc
     // _Pragma is not valid for MSVC, but there isn't any point
     // to handle a _Pragma differently.
   }
 };
 
 } // namespace

clang/lib/Parse/ParsePragma.cpp

 #include "llvm/ADT/StringSwitch.h"
 using namespace clang;
 
 namespace {
 
 struct PragmaAlignHandler : public PragmaHandler {
   explicit PragmaAlignHandler() : PragmaHandler("align") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaGCCVisibilityHandler : public PragmaHandler {
   explicit PragmaGCCVisibilityHandler() : PragmaHandler("visibility") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaOptionsHandler : public PragmaHandler {
   explicit PragmaOptionsHandler() : PragmaHandler("options") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaPackHandler : public PragmaHandler {
   explicit PragmaPackHandler() : PragmaHandler("pack") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaClangSectionHandler : public PragmaHandler {
   explicit PragmaClangSectionHandler(Sema &S)
              : PragmaHandler("section"), Actions(S) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 private:
   Sema &Actions;
 };
 
 struct PragmaMSStructHandler : public PragmaHandler {
   explicit PragmaMSStructHandler() : PragmaHandler("ms_struct") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaUnusedHandler : public PragmaHandler {
   PragmaUnusedHandler() : PragmaHandler("unused") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaWeakHandler : public PragmaHandler {
   explicit PragmaWeakHandler() : PragmaHandler("weak") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaRedefineExtnameHandler : public PragmaHandler {
   explicit PragmaRedefineExtnameHandler() : PragmaHandler("redefine_extname") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaOpenCLExtensionHandler : public PragmaHandler {
   PragmaOpenCLExtensionHandler() : PragmaHandler("EXTENSION") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 
 struct PragmaFPContractHandler : public PragmaHandler {
   PragmaFPContractHandler() : PragmaHandler("FP_CONTRACT") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 // Pragma STDC implementations.
 
 /// PragmaSTDC_FENV_ACCESSHandler - "\#pragma STDC FENV_ACCESS ...".
 struct PragmaSTDC_FENV_ACCESSHandler : public PragmaHandler {
   PragmaSTDC_FENV_ACCESSHandler() : PragmaHandler("FENV_ACCESS") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     tok::OnOffSwitch OOS;
     if (PP.LexOnOffSwitch(OOS))
      return;
     if (OOS == tok::OOS_ON) {
       PP.Diag(Tok, diag::warn_stdc_fenv_access_not_supported);
     }
 
     MutableArrayRef<Token> Toks(PP.getPreprocessorAllocator().Allocate<Token>(1),
                                 1);
     Toks[0].startToken();
     Toks[0].setKind(tok::annot_pragma_fenv_access);
     Toks[0].setLocation(Tok.getLocation());
     Toks[0].setAnnotationEndLoc(Tok.getLocation());
     Toks[0].setAnnotationValue(reinterpret_cast<void*>(
                                static_cast<uintptr_t>(OOS)));
     PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
   }
 };
 
 /// PragmaSTDC_CX_LIMITED_RANGEHandler - "\#pragma STDC CX_LIMITED_RANGE ...".
 struct PragmaSTDC_CX_LIMITED_RANGEHandler : public PragmaHandler {
   PragmaSTDC_CX_LIMITED_RANGEHandler() : PragmaHandler("CX_LIMITED_RANGE") {}
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &Tok) override {
+                    SourceLocation IntroducerLoc, Token &Tok) override {
     tok::OnOffSwitch OOS;
     PP.LexOnOffSwitch(OOS);
   }
 };
 
 /// PragmaSTDC_UnknownHandler - "\#pragma STDC ...".
 struct PragmaSTDC_UnknownHandler : public PragmaHandler {
   PragmaSTDC_UnknownHandler() = default;
 
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &UnknownTok) override {
+                    SourceLocation IntroducerLoc, Token &UnknownTok) override {
     // C99 6.10.6p2, unknown forms are not allowed.
     PP.Diag(UnknownTok, diag::ext_stdc_pragma_ignored);
   }
 };
 
 struct PragmaFPHandler : public PragmaHandler {
   PragmaFPHandler() : PragmaHandler("fp") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaNoOpenMPHandler : public PragmaHandler {
   PragmaNoOpenMPHandler() : PragmaHandler("omp") { }
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaOpenMPHandler : public PragmaHandler {
   PragmaOpenMPHandler() : PragmaHandler("omp") { }
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 /// PragmaCommentHandler - "\#pragma comment ...".
 struct PragmaCommentHandler : public PragmaHandler {
   PragmaCommentHandler(Sema &Actions)
     : PragmaHandler("comment"), Actions(Actions) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 private:
   Sema &Actions;
 };
 
 struct PragmaDetectMismatchHandler : public PragmaHandler {
   PragmaDetectMismatchHandler(Sema &Actions)
     : PragmaHandler("detect_mismatch"), Actions(Actions) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 private:
   Sema &Actions;
 };
 
 struct PragmaMSPointersToMembers : public PragmaHandler {
   explicit PragmaMSPointersToMembers() : PragmaHandler("pointers_to_members") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaMSVtorDisp : public PragmaHandler {
   explicit PragmaMSVtorDisp() : PragmaHandler("vtordisp") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaMSPragma : public PragmaHandler {
   explicit PragmaMSPragma(const char *name) : PragmaHandler(name) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 /// PragmaOptimizeHandler - "\#pragma clang optimize on/off".
 struct PragmaOptimizeHandler : public PragmaHandler {
   PragmaOptimizeHandler(Sema &S)
     : PragmaHandler("optimize"), Actions(S) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 private:
   Sema &Actions;
 };
 
 struct PragmaLoopHintHandler : public PragmaHandler {
   PragmaLoopHintHandler() : PragmaHandler("loop") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaUnrollHintHandler : public PragmaHandler {
   PragmaUnrollHintHandler(const char *name) : PragmaHandler(name) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaMSRuntimeChecksHandler : public EmptyPragmaHandler {
   PragmaMSRuntimeChecksHandler() : EmptyPragmaHandler("runtime_checks") {}
 };
 
 struct PragmaMSIntrinsicHandler : public PragmaHandler {
   PragmaMSIntrinsicHandler() : PragmaHandler("intrinsic") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaMSOptimizeHandler : public PragmaHandler {
   PragmaMSOptimizeHandler() : PragmaHandler("optimize") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 };
 
 struct PragmaForceCUDAHostDeviceHandler : public PragmaHandler {
   PragmaForceCUDAHostDeviceHandler(Sema &Actions)
       : PragmaHandler("force_cuda_host_device"), Actions(Actions) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 
 private:
   Sema &Actions;
 };
 
 /// PragmaAttributeHandler - "\#pragma clang attribute ...".
 struct PragmaAttributeHandler : public PragmaHandler {
   PragmaAttributeHandler(AttributeFactory &AttrFactory)
       : PragmaHandler("attribute"), AttributesForPragmaAttribute(AttrFactory) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                    Token &FirstToken) override;
+                    SourceLocation IntroducerLoc, Token &FirstToken) override;
 
   /// A pool of attributes that were parsed in \#pragma clang attribute.
   ParsedAttributes AttributesForPragmaAttribute;
 };
 
 }  // end namespace
 
 void Parser::initializePragmaHandlers() {
Actions.ActOnPragmaAttributeAttribute(Attribute, PragmaLoc,
                                         std::move(SubjectMatchRules));
 }
 
 // #pragma GCC visibility comes in two variants:
 //   'push' '(' [visibility] ')'
 //   'pop'
 void PragmaGCCVisibilityHandler::HandlePragma(Preprocessor &PP,
                                               PragmaIntroducerKind Introducer,
+                                              SourceLocation IntroducerLoc,
                                               Token &VisTok) {
   SourceLocation VisLoc = VisTok.getLocation();
 
   Token Tok;
   PP.LexUnexpandedToken(Tok);
 
   const IdentifierInfo *PushPop = Tok.getIdentifierInfo();
 

 }
 
 // #pragma pack(...) comes in the following delicious flavors:
 //   pack '(' [integer] ')'
 //   pack '(' 'show' ')'
 //   pack '(' ('push' | 'pop') [',' identifier] [, integer] ')'
 void PragmaPackHandler::HandlePragma(Preprocessor &PP,
                                      PragmaIntroducerKind Introducer,
+                                     SourceLocation IntroducerLoc,
                                      Token &PackTok) {
   SourceLocation PackLoc = PackTok.getLocation();
 
   Token Tok;
   PP.Lex(Tok);
   if (Tok.isNot(tok::l_paren)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen) << "pack";
     return;
void PragmaPackHandler::HandlePragma(Preprocessor &PP,
   Toks[0].setAnnotationValue(static_cast<void*>(Info));
   PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
 }
 
 // #pragma ms_struct on
 // #pragma ms_struct off
 void PragmaMSStructHandler::HandlePragma(Preprocessor &PP,
                                          PragmaIntroducerKind Introducer,
+                                         SourceLocation IntroducerLoc,
                                          Token &MSStructTok) {
   PragmaMSStructKind Kind = PMSST_OFF;
 
   Token Tok;
   PP.Lex(Tok);
   if (Tok.isNot(tok::identifier)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_ms_struct);
     return;
void PragmaMSStructHandler::HandlePragma(Preprocessor &PP,
   Toks[0].setAnnotationEndLoc(EndLoc);
   Toks[0].setAnnotationValue(reinterpret_cast<void*>(
                              static_cast<uintptr_t>(Kind)));
   PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
 }
 
 // #pragma clang section bss="abc" data="" rodata="def" text=""
 void PragmaClangSectionHandler::HandlePragma(Preprocessor &PP,
-             PragmaIntroducerKind Introducer, Token &FirstToken) {
+                                             PragmaIntroducerKind Introducer,
+                                             SourceLocation IntroducerLoc,
+                                             Token &FirstToken) {
 
   Token Tok;
   auto SecKind = Sema::PragmaClangSectionKind::PCSK_Invalid;
 
   PP.Lex(Tok); // eat 'section'
   while (Tok.isNot(tok::eod)) {
     if (Tok.isNot(tok::identifier)) {
       PP.Diag(Tok.getLocation(), diag::err_pragma_expected_clang_section_name) << "clang section";
static void ParseAlignPragma(Preprocessor &PP, Token &FirstTok,
   Toks[0].setAnnotationEndLoc(EndLoc);
   Toks[0].setAnnotationValue(reinterpret_cast<void*>(
                              static_cast<uintptr_t>(Kind)));
   PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
 }
 
 void PragmaAlignHandler::HandlePragma(Preprocessor &PP,
                                       PragmaIntroducerKind Introducer,
+                                      SourceLocation IntroducerLoc,
                                       Token &AlignTok) {
   ParseAlignPragma(PP, AlignTok, /*IsOptions=*/false);
 }
 
 void PragmaOptionsHandler::HandlePragma(Preprocessor &PP,
                                         PragmaIntroducerKind Introducer,
+                                        SourceLocation IntroducerLoc,
                                         Token &OptionsTok) {
   ParseAlignPragma(PP, OptionsTok, /*IsOptions=*/true);
 }
 
 // #pragma unused(identifier)
 void PragmaUnusedHandler::HandlePragma(Preprocessor &PP,
                                        PragmaIntroducerKind Introducer,
+                                       SourceLocation IntroducerLoc,
                                        Token &UnusedTok) {
   // FIXME: Should we be expanding macros here? My guess is no.
   SourceLocation UnusedLoc = UnusedTok.getLocation();
 
   // Lex the left '('.
   Token Tok;
   PP.Lex(Tok);
   if (Tok.isNot(tok::l_paren)) {
void PragmaUnusedHandler::HandlePragma(Preprocessor &PP,
   }
   PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
 }
 
 // #pragma weak identifier
 // #pragma weak identifier '=' identifier
 void PragmaWeakHandler::HandlePragma(Preprocessor &PP,
                                      PragmaIntroducerKind Introducer,
+                                     SourceLocation IntroducerLoc,
                                      Token &WeakTok) {
   SourceLocation WeakLoc = WeakTok.getLocation();
 
   Token Tok;
   PP.Lex(Tok);
   if (Tok.isNot(tok::identifier)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_identifier) << "weak";
     return;
if (HasAlias) {
     pragmaUnusedTok.setLocation(WeakLoc);
     pragmaUnusedTok.setAnnotationEndLoc(WeakLoc);
     Toks[1] = WeakName;
     PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
   }
 }
 
 // #pragma redefine_extname identifier identifier
-void PragmaRedefineExtnameHandler::HandlePragma(Preprocessor &PP,
-                                               PragmaIntroducerKind Introducer,
-                                                Token &RedefToken) {
+void PragmaRedefineExtnameHandler::HandlePragma(
+    Preprocessor &PP, PragmaIntroducerKind Introducer,
+    SourceLocation IntroducerLoc, Token &RedefToken) {
   SourceLocation RedefLoc = RedefToken.getLocation();
 
   Token Tok;
   PP.Lex(Tok);
   if (Tok.isNot(tok::identifier)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_identifier) <<
       "redefine_extname";
     return;
void PragmaRedefineExtnameHandler::HandlePragma(
   pragmaRedefTok.setKind(tok::annot_pragma_redefine_extname);
   pragmaRedefTok.setLocation(RedefLoc);
   pragmaRedefTok.setAnnotationEndLoc(AliasName.getLocation());
   Toks[1] = RedefName;
   Toks[2] = AliasName;
   PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
 }
 
-
-void
-PragmaFPContractHandler::HandlePragma(Preprocessor &PP,
+void PragmaFPContractHandler::HandlePragma(Preprocessor &PP,
                                            PragmaIntroducerKind Introducer,
+                                           SourceLocation IntroducerLoc,
                                            Token &Tok) {
   tok::OnOffSwitch OOS;
   if (PP.LexOnOffSwitch(OOS))
     return;
 
   MutableArrayRef<Token> Toks(PP.getPreprocessorAllocator().Allocate<Token>(1),
                               1);
   Toks[0].startToken();
   Toks[0].setKind(tok::annot_pragma_fp_contract);
   Toks[0].setLocation(Tok.getLocation());
   Toks[0].setAnnotationEndLoc(Tok.getLocation());
   Toks[0].setAnnotationValue(reinterpret_cast<void*>(
                              static_cast<uintptr_t>(OOS)));
   PP.EnterTokenStream(Toks, /*DisableMacroExpansion=*/true);
 }
 
-void
-PragmaOpenCLExtensionHandler::HandlePragma(Preprocessor &PP,
-                                           PragmaIntroducerKind Introducer,
-                                           Token &Tok) {
+void PragmaOpenCLExtensionHandler::HandlePragma(
+    Preprocessor &PP, PragmaIntroducerKind Introducer,
+    SourceLocation IntroducerLoc, Token &Tok) {
   PP.LexUnexpandedToken(Tok);
   if (Tok.isNot(tok::identifier)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_identifier) <<
       "OPENCL";
     return;
   }
   IdentifierInfo *Ext = Tok.getIdentifierInfo();
   SourceLocation NameLoc = Tok.getLocation();
void PragmaOpenCLExtensionHandler::HandlePragma(
 
   if (PP.getPPCallbacks())
     PP.getPPCallbacks()->PragmaOpenCLExtension(NameLoc, Ext,
                                                StateLoc, State);
 }
 
 /// Handle '#pragma omp ...' when OpenMP is disabled.
 ///
-void
-PragmaNoOpenMPHandler::HandlePragma(Preprocessor &PP,
+void PragmaNoOpenMPHandler::HandlePragma(Preprocessor &PP,
                                          PragmaIntroducerKind Introducer,
+                                         SourceLocation IntroducerLoc,
                                          Token &FirstTok) {
   if (!PP.getDiagnostics().isIgnored(diag::warn_pragma_omp_ignored,
                                      FirstTok.getLocation())) {
     PP.Diag(FirstTok, diag::warn_pragma_omp_ignored);
     PP.getDiagnostics().setSeverity(diag::warn_pragma_omp_ignored,
                                     diag::Severity::Ignored, SourceLocation());
   }
   PP.DiscardUntilEndOfDirective();
 }
 
 /// Handle '#pragma omp ...' when OpenMP is enabled.
 ///
-void
-PragmaOpenMPHandler::HandlePragma(Preprocessor &PP,
+void PragmaOpenMPHandler::HandlePragma(Preprocessor &PP,
                                        PragmaIntroducerKind Introducer,
+                                       SourceLocation IntroducerLoc,
                                        Token &FirstTok) {
   SmallVector<Token, 16> Pragma;
   Token Tok;
   Tok.startToken();
   Tok.setKind(tok::annot_pragma_openmp);
   Tok.setLocation(FirstTok.getLocation());
 
   while (Tok.isNot(tok::eod) && Tok.isNot(tok::eof)) {
     Pragma.push_back(Tok);

 //
 // <inheritance model> ::= ('single' | 'multiple' | 'virtual') '_inheritance'
 //
 // #pragma pointers_to_members '(' 'best_case' ')'
 // #pragma pointers_to_members '(' 'full_generality' [',' inheritance-model] ')'
 // #pragma pointers_to_members '(' inheritance-model ')'
 void PragmaMSPointersToMembers::HandlePragma(Preprocessor &PP,
                                              PragmaIntroducerKind Introducer,
+                                             SourceLocation IntroducerLoc,
                                              Token &Tok) {
   SourceLocation PointersToMembersLoc = Tok.getLocation();
   PP.Lex(Tok);
   if (Tok.isNot(tok::l_paren)) {
     PP.Diag(PointersToMembersLoc, diag::warn_pragma_expected_lparen)
       << "pointers_to_members";
     return;
   }

 //
 // <vtordisp-mode> ::= ('off' | 'on' | '0' | '1' | '2' )
 //
 // #pragma vtordisp '(' ['push' ','] vtordisp-mode ')'
 // #pragma vtordisp '(' 'pop' ')'
 // #pragma vtordisp '(' ')'
 void PragmaMSVtorDisp::HandlePragma(Preprocessor &PP,
                                     PragmaIntroducerKind Introducer,
-                                    Token &Tok) {
+                                    SourceLocation IntroducerLoc, Token &Tok) {
   SourceLocation VtorDispLoc = Tok.getLocation();
   PP.Lex(Tok);
   if (Tok.isNot(tok::l_paren)) {
     PP.Diag(VtorDispLoc, diag::warn_pragma_expected_lparen) << "vtordisp";
     return;
   }
   PP.Lex(Tok);
 
AnnotTok.setAnnotationValue(reinterpret_cast<void *>(
       static_cast<uintptr_t>((Action << 16) | (Value & 0xFFFF))));
   PP.EnterToken(AnnotTok);
 }
 
 /// Handle all MS pragmas.  Simply forwards the tokens after inserting
 /// an annotation token.
 void PragmaMSPragma::HandlePragma(Preprocessor &PP,
                                   PragmaIntroducerKind Introducer,
-                                  Token &Tok) {
+                                  SourceLocation IntroducerLoc, Token &Tok) {
   Token EoF, AnnotTok;
   EoF.startToken();
   EoF.setKind(tok::eof);
   AnnotTok.startToken();
   AnnotTok.setKind(tok::annot_pragma_ms_pragma);
   AnnotTok.setLocation(Tok.getLocation());
   AnnotTok.setAnnotationEndLoc(Tok.getLocation());
   SmallVector<Token, 8> TokenVector;

 ///   #pragma detect_mismatch("name", "value")
 /// \endcode
 /// Where 'name' and 'value' are quoted strings.  The values are embedded in
 /// the object file and passed along to the linker.  If the linker detects a
 /// mismatch in the object file's values for the given name, a LNK2038 error
 /// is emitted.  See MSDN for more details.
 void PragmaDetectMismatchHandler::HandlePragma(Preprocessor &PP,
                                                PragmaIntroducerKind Introducer,
+                                               SourceLocation IntroducerLoc,
                                                Token &Tok) {
   SourceLocation DetectMismatchLoc = Tok.getLocation();
   PP.Lex(Tok);
   if (Tok.isNot(tok::l_paren)) {
     PP.Diag(DetectMismatchLoc, diag::err_expected) << tok::l_paren;
     return;
   }
 

 /// \code
 ///   #pragma comment(linker, "foo")
 /// \endcode
 /// 'linker' is one of five identifiers: compiler, exestr, lib, linker, user.
 /// "foo" is a string, which is fully macro expanded, and permits string
 /// concatenation, embedded escape characters etc.  See MSDN for more details.
 void PragmaCommentHandler::HandlePragma(Preprocessor &PP,
                                         PragmaIntroducerKind Introducer,
+                                        SourceLocation IntroducerLoc,
                                         Token &Tok) {
   SourceLocation CommentLoc = Tok.getLocation();
   PP.Lex(Tok);
   if (Tok.isNot(tok::l_paren)) {
     PP.Diag(CommentLoc, diag::err_pragma_comment_malformed);
     return;
   }
 
if (PP.getPPCallbacks())
     PP.getPPCallbacks()->PragmaComment(CommentLoc, II, ArgumentString);
 
   Actions.ActOnPragmaMSComment(CommentLoc, Kind, ArgumentString);
 }
 
 // #pragma clang optimize off
 // #pragma clang optimize on
 void PragmaOptimizeHandler::HandlePragma(Preprocessor &PP,
                                          PragmaIntroducerKind Introducer,
+                                         SourceLocation IntroducerLoc,
                                          Token &FirstToken) {
   Token Tok;
   PP.Lex(Tok);
   if (Tok.is(tok::eod)) {
     PP.Diag(Tok.getLocation(), diag::err_pragma_missing_argument)
         << "clang optimize" << /*Expected=*/true << "'on' or 'off'";
     return;
   }
   if (Tok.isNot(tok::identifier)) {
struct TokFPAnnotValue {
 
   FlagKinds FlagKind;
   FlagValues FlagValue;
 };
 } // end anonymous namespace
 
 void PragmaFPHandler::HandlePragma(Preprocessor &PP,
                                    PragmaIntroducerKind Introducer,
-                                   Token &Tok) {
+                                   SourceLocation IntroducerLoc, Token &Tok) {
   // fp
   Token PragmaName = Tok;
   SmallVector<Token, 1> TokenList;
 
   PP.Lex(Tok);
   if (Tok.isNot(tok::identifier)) {
     PP.Diag(Tok.getLocation(), diag::err_pragma_fp_invalid_option)
         << /*MissingOption=*/true << "";

 /// completely if the trip count is known at compile time and unroll partially
 /// if the trip count is not known.  Specifying unroll(full) is similar to
 /// unroll(enable) but will unroll the loop only if the trip count is known at
 /// compile time.  Specifying unroll(disable) disables unrolling for the
 /// loop. Specifying unroll_count(_value_) instructs llvm to try to unroll the
 /// loop the number of times indicated by the value.
 void PragmaLoopHintHandler::HandlePragma(Preprocessor &PP,
                                          PragmaIntroducerKind Introducer,
+                                         SourceLocation IntroducerLoc,
                                          Token &Tok) {
   // Incoming token is "loop" from "#pragma clang loop".
   Token PragmaName = Tok;
   SmallVector<Token, 1> TokenList;
 
   // Lex the optimization option and verify it is an identifier.
   PP.Lex(Tok);
   if (Tok.isNot(tok::identifier)) {

 /// contained in parentheses. With no argument the directive instructs llvm to
 /// try to unroll the loop completely. A positive integer argument can be
 /// specified to indicate the number of times the loop should be unrolled.  To
 /// maximize compatibility with other compilers the unroll count argument can be
 /// specified with or without parentheses.  Specifying, '#pragma nounroll'
 /// disables unrolling of the loop.
 void PragmaUnrollHintHandler::HandlePragma(Preprocessor &PP,
                                            PragmaIntroducerKind Introducer,
+                                           SourceLocation IntroducerLoc,
                                            Token &Tok) {
   // Incoming token is "unroll" for "#pragma unroll", or "nounroll" for
   // "#pragma nounroll".
   Token PragmaName = Tok;
   PP.Lex(Tok);
   auto *Info = new (PP.getPreprocessorAllocator()) PragmaLoopHintInfo;
   if (Tok.is(tok::eod)) {
     // nounroll or unroll pragma without an argument.

 /// \endcode
 ///
 /// Pragma intrisic tells the compiler to use a builtin version of the
 /// function. Clang does it anyway, so the pragma doesn't really do anything.
 /// Anyway, we emit a warning if the function specified in \#pragma intrinsic
 /// isn't an intrinsic in clang and suggest to include intrin.h.
 void PragmaMSIntrinsicHandler::HandlePragma(Preprocessor &PP,
                                             PragmaIntroducerKind Introducer,
+                                            SourceLocation IntroducerLoc,
                                             Token &Tok) {
   PP.Lex(Tok);
 
   if (Tok.isNot(tok::l_paren)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen)
         << "intrinsic";
     return;
   }
void PragmaMSIntrinsicHandler::HandlePragma(Preprocessor &PP,
   if (Tok.isNot(tok::eod))
     PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
         << "intrinsic";
 }
 
 // #pragma optimize("gsty", on|off)
 void PragmaMSOptimizeHandler::HandlePragma(Preprocessor &PP,
                                            PragmaIntroducerKind Introducer,
+                                           SourceLocation IntroducerLoc,
                                            Token &Tok) {
   SourceLocation StartLoc = Tok.getLocation();
   PP.Lex(Tok);
 
   if (Tok.isNot(tok::l_paren)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen) << "optimize";
     return;
   }
if (Tok.isNot(tok::eod)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
         << "optimize";
     return;
   }
   PP.Diag(StartLoc, diag::warn_pragma_optimize);
 }
 
 void PragmaForceCUDAHostDeviceHandler::HandlePragma(
-    Preprocessor &PP, PragmaIntroducerKind Introducer, Token &Tok) {
+    Preprocessor &PP, PragmaIntroducerKind Introducer,
+    SourceLocation IntroducerLoc, Token &Tok) {
   Token FirstTok = Tok;
 
   PP.Lex(Tok);
   IdentifierInfo *Info = Tok.getIdentifierInfo();
   if (!Info || (!Info->isStr("begin") && !Info->isStr("end"))) {
     PP.Diag(FirstTok.getLocation(),
             diag::warn_pragma_force_cuda_host_device_bad_arg);
     return;

 /// attribute. Its exact syntax is described in the LanguageExtensions document
 /// in Clang's documentation.
 ///
 /// This directive instructs the compiler to begin/finish applying the specified
 /// attribute to the set of attribute-specific declarations in the active range
 /// of the pragma.
 void PragmaAttributeHandler::HandlePragma(Preprocessor &PP,
                                           PragmaIntroducerKind Introducer,
+                                          SourceLocation IntroducerLoc,
                                           Token &FirstToken) {
   Token Tok;
   PP.Lex(Tok);
   auto *Info = new (PP.getPreprocessorAllocator())
       PragmaAttributeInfo(AttributesForPragmaAttribute);
 
   // Parse the optional namespace followed by a period.
   if (Tok.is(tok::identifier)) {