[clang] Add a diagnostic for line directive of a gnu extension

[ken-matsui]

Summary

This change is related to: https://github.com/llvm/llvm-project/issues/55067

[ken-matsui]

This comment was removed by ken-matsui.

[aaron.ballman]

Thank you for working on this new diagnostic!

clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp

1 ↗

(On Diff #425537)

I think this file (and the next one) are making unintended line ending changes and should be reverted.

clang/include/clang/Basic/DiagnosticLexKinds.td

430

Tweaking for grammar, but mostly, this should be marked as an Extension rather than a Warning so that it's controlled only via -pedantic rather than another warning group.

clang/test/Misc/warning-flags.c

21 ↗

(On Diff #425537)

Once you make the diagnostic an extension, the changes to this file can be reverted. (It's worth noting that two lines above we document that this list should not grow.)

clang/test/Preprocessor/line-directive.c

30–31

This should get the same diagnostic.

35

As should this form.

[ken-matsui]

Add a diagnostic for line directive of a gnu extension

[ken-matsui]

Hi @aaron.ballman,

Thank you for your review! I updated the code.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticLexKinds.td

431

Oops, I missed this last time around, but we need this to be in a warning group as well.

I think you should add: def GNULineMarker : DiagGroup<"gnu-line-marker">; to DiagnosticGroups.td and then add GNULineMarker to the GNU warning group.

Then here, you can make these changes.

clang/lib/Lex/PPDirectives.cpp

1401

I think you need to add another call to Diag() here.

clang/test/Misc/warning-flags.c

93 ↗

(On Diff #425601)

Oops, I missed this before -- two lines up, it also says this count should never grow. This should be fixed by my suggestion to add it to a warning group.

clang/test/Preprocessor/line-directive.c

35

It looks like this form isn't getting the warning we'd expect.

119–120

These should also get the warning.

[ken-matsui]

Add a diagnostic for line directive of a gnu extension

[ken-matsui]

Hi, @aaron.ballman

Thanks for your kind reviews!
I’ve updated the code, but is this what you’d expected?

[aaron.ballman]

In D124534#3481264, @ken-matsui wrote:

Hi, @aaron.ballman

Thanks for your kind reviews!
I’ve updated the code, but is this what you’d expected?

It looks like the precommit CI pipeline can't test your patch -- I think you may have uploaded just the changes since last time rather than the full set of changes against the tree. I don't think we're quite there yet, but we're not too far off either, I think. Thank you for continuing to work on this!

clang/include/clang/Basic/DiagnosticGroups.td

1116–1118

You'll likely also need to re-flow the line to the usual 80-col limit.

clang/lib/Lex/PPDirectives.cpp

1350

I speculate that this change is wrong.

The goal here is to diagnose any time there's a GNU line marker and now we're only trigging the diagnostic when the line marker's value is 1; that misses diagnostics when the marker value is something else.

That's why I suggested warning each place we return false from this function -- those are the situations when the line marker is syntactically correct and we're going to make use of it in the caller. (We don't want to warn about use of a line marker when we're going to generate an error anyway.)

[ken-matsui]

clang/lib/Lex/PPDirectives.cpp

1350

@aaron.ballman

Thank you!

Just to confirm, do I need to remove the call of Diag after GetLineValue and put Diags into all branches of returning false in this function?
If so, I think putting Diag after the call of this function would be better.

[aaron.ballman]

clang/lib/Lex/PPDirectives.cpp

1350

If so, I think putting Diag after the call of this function would be better.

You are correct and I agree, good suggestion!

[ken-matsui]

clang/lib/Lex/PPDirectives.cpp

1350

@aaron.ballman
Thank you for your response!

I've updated the code as mentioned, but a bunch of other tests with the -pedantic option failed as the following warnings:

******************** TEST 'Clang :: CXX/expr/expr.const/p2-0x.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   /tmp/llvm/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -fsyntax-only -std=c++11 -pedantic -verify=expected,cxx11 -fcxx-exceptions /tmp/llvm/llvm-project/clang/test/CXX/expr/expr.const/p2-0x.cpp -fconstexpr-depth 128 -triple i686-pc-linux-gnu
: 'RUN: at line 2';   /tmp/llvm/llvm-project/build/bin/clang -cc1 -internal-isystem /tmp/llvm/llvm-project/build/lib/clang/15.0.0/include -nostdsysteminc -fsyntax-only -std=c++2a -pedantic -verify=expected,cxx20 -fcxx-exceptions /tmp/llvm/llvm-project/clang/test/CXX/expr/expr.const/p2-0x.cpp -fconstexpr-depth 128 -triple i686-pc-linux-gnu
--
Exit Code: 1

Command Output (stderr):
--
error: 'warning' diagnostics seen but not expected: 
  Line 0: this style of line directive is a GNU extension
  Line 0: this style of line directive is a GNU extension
2 errors generated.

...

I personally think it would be preferable if the only change of tests would be line-directive.c.
So, how about reducing Diag calls until the warning doesn't spill over into other tests?

[ken-matsui]

Add a diagnostic for line directive of a gnu extension

[aaron.ballman]

clang/lib/Lex/PPDirectives.cpp

1350

I personally think it would be preferable if the only change of tests would be line-directive.c.
So, how about reducing Diag calls until the warning doesn't spill over into other tests?

No, this is expected. We're adding a diagnostic where there wasn't one previously, so some files are going to get caught by that. You can either add the // expected-warning {{}} comments to those lines, or if the test has a lot of those lines but isn't really specific to line markers (it just happens to use them to test other functionality) you can disable the diagnostic for that test with -Wno-gnu-line-marker.

[ken-matsui]

clang/lib/Lex/PPDirectives.cpp

1350

Ah, I see. I'm going to work on it. Thank you!

[ken-matsui]

@aaron.ballman

If so, I think putting Diag after the call of this function would be better.

With the above change, I tried to add comments to failed tests, but there were over 300 files.

During my investigation, I found most tests printed warnings without file information strangely.

error: 'warning' diagnostics seen but not expected: 
  Line 0: this style of line directive is a GNU extension
  Line 0: this style of line directive is a GNU extension
2 errors generated.

If warnings have file information, it should be like:

error: 'warning' diagnostics seen but not expected: 
  File /tmp/llvm-project/clang/test/Preprocessor/line-directive.c Line 9: this style of line directive is a GNU extension
1 error generated.

Even tests that completely do not use preprocessor directives, such as clang/test/SemaCXX/matrix-type.cpp, failed with the above strange warnings.

Thus, I suspect that the warnings without file paths have come from internally included SDK (I'm using macOS that includes /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk) or something that is not related to target test files.

What do you think?

[aaron.ballman]

In D124534#3490098, @ken-matsui wrote:

@aaron.ballman

If so, I think putting Diag after the call of this function would be better.

With the above change, I tried to add comments to failed tests, but there were over 300 files.

During my investigation, I found most tests printed warnings without file information strangely.

error: 'warning' diagnostics seen but not expected: 
  Line 0: this style of line directive is a GNU extension
  Line 0: this style of line directive is a GNU extension
2 errors generated.

That is rather unexpected.

If warnings have file information, it should be like:

error: 'warning' diagnostics seen but not expected: 
  File /tmp/llvm-project/clang/test/Preprocessor/line-directive.c Line 9: this style of line directive is a GNU extension
1 error generated.

Even tests that completely do not use preprocessor directives, such as clang/test/SemaCXX/matrix-type.cpp, failed with the above strange warnings.

Thus, I suspect that the warnings without file paths have come from internally included SDK (I'm using macOS that includes /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk) or something that is not related to target test files.

What do you think?

The tests run in -cc1 mode and don't #include anything, so I don't think the issue is an internally included SDK. I think the issue could be from this: https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/InitPreprocessor.cpp#L1355 and https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/InitPreprocessor.cpp#L1368. You may have to hook up to a debugger to see why we're issuing those surprising warnings. If it turns out that it's these inserted directives, you may have to look at the source location of the digit token to see if isWrittenInBuiltinFile() is true or not (and we may need to also check isWrittenInScratchSpace() as well, perhaps).

[ken-matsui]

In D124534#3490732, @aaron.ballman wrote:

The tests run in -cc1 mode and don't #include anything, so I don't think the issue is an internally included SDK. I think the issue could be from this: https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/InitPreprocessor.cpp#L1355 and https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/InitPreprocessor.cpp#L1368. You may have to hook up to a debugger to see why we're issuing those surprising warnings. If it turns out that it's these inserted directives, you may have to look at the source location of the digit token to see if isWrittenInBuiltinFile() is true or not (and we may need to also check isWrittenInScratchSpace() as well, perhaps).

Thank you for the information! I'm going to look into the source code.

[ken-matsui]

Update codes as reviewed

[ken-matsui]

@aaron.ballman

I could suppress the strange warnings by using isWrittenInBuiltinFile and isWrittenInCommandLineFile. Thank you!
Could you please review this patch?

[aaron.ballman]

It looks like there's still something not quite right because it's warning on some instances but not others and I'm not certain why. Can you investigate?

clang/test/Preprocessor/line-directive.c

35–36

Something is still not quite correct -- these should also be diagnosed as an extension (it's the same feature just with flags).

63

This should also be diagnosed.

[ken-matsui]

@aaron.ballman

Thank you for your review and sorry to have missed those directives!

I also found additional directives that I might have to diagnose.
Could you please tell me if these are also required?

clang/test/Preprocessor/line-directive.c

69–79

Should these directives also be diagnosed?

[ken-matsui]

After some investigations, some directives behave weirdly.
I will continue investigating, but do you have any suggestions here?

clang/test/Preprocessor/line-directive.c

35

This didn't cause a warning on this test file, but another test file I created caused a warning.

// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -pedantic %s

# 42 "foo" 1 3  // enter: expected-warning {{this style of line directive is a GNU extension}}

63

This directive didn't cause a warning when marking both # 192 "glomp.h" and this. Marking only either directive works.

Works:

...
# 192 "glomp.h" // not a system header.: expected-warning {{this style of line directive is a GNU extension}}
typedef int x;  // expected-note {{previous definition is here}}
typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 feature}}

# 192 "glomp.h" 3 // System header.
...

Works:

...
# 192 "glomp.h" // not a system header.
typedef int x;  // expected-note {{previous definition is here}}
typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 feature}}

# 192 "glomp.h" 3 // System header.: expected-warning {{this style of line directive is a GNU extension}}
...

Does NOT work:

...
# 192 "glomp.h" // not a system header.: expected-warning {{this style of line directive is a GNU extension}}
typedef int x;  // expected-note {{previous definition is here}}
typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 feature}}

# 192 "glomp.h" 3 // System header.: expected-warning {{this style of line directive is a GNU extension}}
...

Command Output (stderr):
--
error: 'warning' diagnostics expected but not seen: 
  File /tmp/llvm/llvm-project/clang/test/Preprocessor/line-directive.c Line 191: this style of line directive is a GNU extension
1 error generated.

[aaron.ballman]

clang/test/Preprocessor/line-directive.c

35

Oooohhhh, wait a tick! This is entering a system header! Perhaps we're silencing the warning because it's "in" a system header!

That's a neat edge case for us to think about -- do users of this diagnostic *expect* such a construct to be diagnosed? What about the exit from the system header? GCC seems to diagnose the entrance to the system header, but not the exit: https://godbolt.org/z/PKGd4jh64 and I don't know if we want to follow their behavior or not. Our current behavior is defensible, if it marks the entrance to a system header or the exit from a system header, we're silent. User who want to see the system header diagnostics can use -Wsystem-headers to get them. So I think I've talked myself into the current behavior here being correct -- but we should probably add a RUN line that enables diagnostics in system headers to show our behavior there.

69–79

Yes, but this may be a case of the directive being in a system header and thus the diagnostic is suppressed.

[ken-matsui]

Add a test file that uses the -Wsystem-headers option.

[ken-matsui]

clang/test/Preprocessor/line-directive.c

35

Ah, I'm quite not familiar with a preprocessor, so I couldn't have understood what was going on. Thank you for your clear explanation.

I've added a new file that tests with -Wsystem-headers. This test shows that it seems mostly correct expectations.

[aaron.ballman]

Thanks, I think we're getting pretty close!

clang/test/Preprocessor/line-directive-system-headers.c

2–6

I think we're losing a bunch of test coverage from this. Why did you drop these?

57–58

It took me a moment, but we get these diagnostics because we asked for warnings in a system header. So I think we're losing test coverage from this change -- previously the test was checking that we suppressed the diagnostics as expected.

I think you should have two RUN lines, one with -Wsystem-headers and one without, so you can test the behavior of both situations. Note, you can do -verify=system on the one with -Wsystem-headers so that you can write // system-warning {{whatever}} on the diagnostics expected to only appear from that RUN line.

Alternatively, you could make this test solely about system header behavior and modify line-directive.c to not do anything with system headers. But I think that's a much more invasive change.

clang/test/Preprocessor/line-directive.c

35–36

[ken-matsui]

Thank you for your review!

clang/test/Preprocessor/line-directive-system-headers.c

2–6

These are also tested on line-directive.c and possibly redundant, so I removed them.
Should I keep them?

57–58

Thank you! I'm going to edit this file, not line-directive.c.

[aaron.ballman]

clang/test/Preprocessor/line-directive-system-headers.c

2–6

You're right, those are redundant; I'm fine removing them here.

[ken-matsui]

clang/test/Preprocessor/line-directive-system-headers.c

2–6

I see 👍

[ken-matsui]

Update test codes as reviewed

[aaron.ballman]

clang/test/Preprocessor/line-directive-system-headers.c

2–3

There's nothing specific to C99 that I can see in the test, so I think these can be dropped.

20–22

Might as well make this consistent with the other parts of the test and use @#6 instead of @-1. Same applies elsewhere.

41–51

Let's change "no expected warning" into "Warnings silenced when -Wsystem-headers isn't passed." so it sounds less like "we don't expect the warning directly below this comment to happen".

[ken-matsui]

Thank you for your review! I'm going to fix them :)

[ken-matsui]

Fix the test as reviewed

[aaron.ballman]

LGTM, thank you for all the hard work on this! I assume you need me to land this on your behalf -- if so, can you let me know what name and email address you would like me to use for patch attribution?

I think we should add a release note for this to let folks know about the new warning group (there's a "Improvements to Clang's diagnostics" in clang/docs/ReleaseNotes.rst). You can either add one to this patch, or I can add one for you when I go to land, I'm fine either way.

[ken-matsui]

In D124534#3504610, @aaron.ballman wrote:

LGTM, thank you for all the hard work on this! I assume you need me to land this on your behalf -- if so, can you let me know what name and email address you would like me to use for patch attribution?

Thank you so much for your support and approval!

Yes, I do not have commit access; could you please use the following info for this patch?

Name: Ken Matsui
Email: [e-mail]

(I use this info for every commit on GitHub, but is this what you expected? Please let me know if you need an email where you can be reached out.)

I think we should add a release note for this to let folks know about the new warning group (there's a "Improvements to Clang's diagnostics" in clang/docs/ReleaseNotes.rst). You can either add one to this patch, or I can add one for you when I go to land, I'm fine either way.

Could you please add it to this patch for this time?
I would like to add one next time by referring to how you do.

[aaron.ballman]

In D124534#3504641, @ken-matsui wrote:

In D124534#3504610, @aaron.ballman wrote:

LGTM, thank you for all the hard work on this! I assume you need me to land this on your behalf -- if so, can you let me know what name and email address you would like me to use for patch attribution?

Thank you so much for your support and approval!

Happy to help!

Yes, I do not have commit access; could you please use the following info for this patch?

Name: Ken Matsui
Email: [e-mail]

(I use this info for every commit on GitHub, but is this what you expected? Please let me know if you need an email where you can be reached out.)

That looks to be exactly what I needed, but you can double-check that you were properly attributed here: https://github.com/llvm/llvm-project/commit/786c721c2bbd2e0646e314671e010859550423bf

I think we should add a release note for this to let folks know about the new warning group (there's a "Improvements to Clang's diagnostics" in clang/docs/ReleaseNotes.rst). You can either add one to this patch, or I can add one for you when I go to land, I'm fine either way.

Could you please add it to this patch for this time?
I would like to add one next time by referring to how you do.

I added one when I landed. I also adjusted the commit message slightly from what you have here in Phab for some added clarity.

[aaron.ballman]

In D124534#3505709, @aaron.ballman wrote:

In D124534#3504641, @ken-matsui wrote:

(I use this info for every commit on GitHub, but is this what you expected? Please let me know if you need an email where you can be reached out.)

That looks to be exactly what I needed, but you can double-check that you were properly attributed here: https://github.com/llvm/llvm-project/commit/786c721c2bbd2e0646e314671e010859550423bf

Actually, it does raise an interesting issue -- I don't think you'll get build failure emails when using that address. After a patch is landed, a bunch of build bots test the patch out and when there's a failure, they send email to everyone on the blame list. You should probably use an email address that receives email for future commits (or when you go to obtain your own commit privileges) so that you can receive those emails.

[ken-matsui]

That looks to be exactly what I needed, but you can double-check that you were properly attributed here: https://github.com/llvm/llvm-project/commit/786c721c2bbd2e0646e314671e010859550423bf
I added one when I landed. I also adjusted the commit message slightly from what you have here in Phab for some added clarity.

Thank you! I was able to confirm that I was attributed.

In D124534#3505729, @aaron.ballman wrote:

In D124534#3505709, @aaron.ballman wrote:

In D124534#3504641, @ken-matsui wrote:

(I use this info for every commit on GitHub, but is this what you expected? Please let me know if you need an email where you can be reached out.)

That looks to be exactly what I needed, but you can double-check that you were properly attributed here: https://github.com/llvm/llvm-project/commit/786c721c2bbd2e0646e314671e010859550423bf

Actually, it does raise an interesting issue -- I don't think you'll get build failure emails when using that address. After a patch is landed, a bunch of build bots test the patch out and when there's a failure, they send email to everyone on the blame list. You should probably use an email address that receives email for future commits (or when you go to obtain your own commit privileges) so that you can receive those emails.

Ah, sorry. I also have a public email for GitHub (just mostly used the private one), so I'll share a public one in the future.

[Jake-Egan]

Hi, this patch causes a test failure on AIX https://lab.llvm.org/buildbot/#/builders/214/builds/1221/steps/6/logs/FAIL__Clang__noinline_cu

Could you please make a fix or revert if it takes too long?

[ken-matsui]

Hello @Jake-Egan,

I believe the problem was already fixed by the @aaron.ballman's patch here.

Thank you for fixing it, @aaron.ballman!

[shafik]

Looks like this change is coming up in: https://github.com/llvm/llvm-project/issues/63284

clang/docs/ReleaseNotes.rst

- ``-Wthread-safety-analysis`` now considers overloaded compound assignment and
   requiring an exclusive lock if the argument is guarded.
 - ``-Wenum-conversion`` now warns on converting a signed enum of one type to an
   unsigned enum of a different type (or vice versa) rather than
   ``-Wsign-conversion``.
 - Added the ``-Wunreachable-code-generic-assoc`` diagnostic flag (grouped under
   the ``-Wunreachable-code`` flag) which is enabled by default and warns the
   user about ``_Generic`` selection associations which are unreachable because
   the type specified is an array type or a qualified type.
+- Added the ``-Wgnu-line-marker`` diagnostic flag (grouped under the ``-Wgnu``
+  flag) which is a portability warning about use of GNU linemarker preprocessor
+  directives. Fixes `Issue 55067 <https://github.com/llvm/llvm-project/issues/55067>`_.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 
 - Improve __builtin_dump_struct:
 
   - Support bitfields in struct and union.
   - Improve the dump format, dump both bitwidth(if its a bitfield) and field

clang/include/clang/Basic/DiagnosticGroups.td

 def InvalidSourceEncoding : DiagGroup<"invalid-source-encoding">;
 def KNRPromotedParameter : DiagGroup<"knr-promoted-parameter">;
 def DeprecatedNonPrototype : DiagGroup<"deprecated-non-prototype">;
 def StrictPrototypes : DiagGroup<"strict-prototypes", [DeprecatedNonPrototype]>;
 def : DiagGroup<"init-self">;
 def : DiagGroup<"inline">;
 def : DiagGroup<"invalid-pch">;
 def GNULabelsAsValue : DiagGroup<"gnu-label-as-value">;
+def GNULineMarker : DiagGroup<"gnu-line-marker">;
 def LiteralRange : DiagGroup<"literal-range">;
 def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
                                       [CXX98CompatLocalTypeTemplateArgs]>;
 def RangeLoopConstruct : DiagGroup<"range-loop-construct">;
 def RangeLoopBindReference : DiagGroup<"range-loop-bind-reference">;
 def RangeLoopAnalysis : DiagGroup<"range-loop-analysis",
                                   [RangeLoopConstruct, RangeLoopBindReference]>;
 def ForLoopAnalysis : DiagGroup<"for-loop-analysis">;

 def GNU : DiagGroup<"gnu", [GNUAlignofExpression, GNUAnonymousStruct,
                             GNUAutoType,
                             GNUBinaryLiteral, GNUCaseRange,
                             GNUComplexInteger, GNUCompoundLiteralInitializer,
                             GNUConditionalOmittedOperand, GNUDesignator,
                             GNUEmptyInitializer, GNUEmptyStruct,
                             VLAExtension, GNUFlexibleArrayInitializer,
                             GNUFlexibleArrayUnionMember, GNUFoldingConstant,
                             GNUImaginaryConstant, GNUIncludeNext,
-                            GNULabelsAsValue, GNUNullPointerArithmetic,
+                            GNULabelsAsValue, GNULineMarker, GNUNullPointerArithmetic,
                             GNUPointerArith, RedeclaredClassMember,
                             GNURedeclaredEnum, GNUStatementExpression,
                             GNUStaticFloatInit,
                             GNUStringLiteralOperatorTemplate, GNUUnionCast,
                             GNUVariableSizedTypeNotAtEnd, ZeroLengthArray,
                             GNUZeroLineDirective,
                             GNUZeroVariadicMacroArguments]>;
 // A warning group for warnings about code that clang accepts but gcc doesn't.
 def GccCompat : DiagGroup<"gcc-compat">;

clang/include/clang/Basic/DiagnosticLexKinds.td

def warn_cxx98_compat_empty_fnmacro_arg : Warning<
   "empty macro arguments are incompatible with C++98">,
   InGroup<CXX98CompatPedantic>, DefaultIgnore;
 def note_macro_here : Note<"macro %0 defined here">;
 def note_macro_expansion_here : Note<"expansion of macro %0 requested here">;
 
 def ext_pp_opencl_variadic_macros : Extension<
   "variadic macros are a Clang extension in OpenCL">;
 
+def ext_pp_gnu_line_directive : Extension<
+  "this style of line directive is a GNU extension">,
+  InGroup<GNULineMarker>;
+
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<
   "%0 must be used within a preprocessing directive">;
 def err_pp_file_not_found : Error<"'%0' file not found">, DefaultFatal;
 def err_pp_through_header_not_found : Error<
   "'%0' required for precompiled header not found">, DefaultFatal;
 def err_pp_through_header_not_seen : Error<
   "#include of '%0' not seen while attempting to "

clang/lib/Lex/PPDirectives.cpp

static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
   if (FlagVal == 1) {
     IsFileEntry = true;
 
     PP.Lex(FlagTok);
     if (FlagTok.is(tok::eod)) return false;
     if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag,PP))
       return true;
   } else if (FlagVal == 2) {
     IsFileExit = true;
 
     SourceManager &SM = PP.getSourceManager();
     // If we are leaving the current presumed file, check to make sure the
     // presumed include stack isn't empty!
     FileID CurFileID =
       SM.getDecomposedExpansionLoc(FlagTok.getLocation()).first;
     PresumedLoc PLoc = SM.getPresumedLoc(FlagTok.getLocation());
     if (PLoc.isInvalid())
if (FlagVal != 4) {
     PP.Diag(FlagTok, diag::err_pp_linemarker_invalid_flag);
     PP.DiscardUntilEndOfDirective();
     return true;
   }
 
   FileKind = SrcMgr::C_ExternCSystem;
 
   PP.Lex(FlagTok);
   if (FlagTok.is(tok::eod)) return false;
 
   // There are no more valid flags here.
   PP.Diag(FlagTok, diag::err_pp_linemarker_invalid_flag);
   PP.DiscardUntilEndOfDirective();
   return true;
 }
 
 /// HandleDigitDirective - Handle a GNU line marker directive, whose syntax is
void Preprocessor::HandleDigitDirective(Token &DigitTok) {
 
   bool IsFileEntry = false, IsFileExit = false;
   int FilenameID = -1;
   SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
 
   // If the StrTok is "eod", then it wasn't present.  Otherwise, it must be a
   // string followed by eod.
   if (StrTok.is(tok::eod)) {
+    Diag(StrTok, diag::ext_pp_gnu_line_directive);
     // Treat this like "#line NN", which doesn't change file characteristics.
     FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
   } else if (StrTok.isNot(tok::string_literal)) {
     Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
     DiscardUntilEndOfDirective();
     return;
   } else if (StrTok.hasUDSuffix()) {
     Diag(StrTok, diag::err_invalid_string_udl);
if (Literal.Pascal) {
       Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
       DiscardUntilEndOfDirective();
       return;
     }
 
     // If a filename was present, read any flags that are present.
     if (ReadLineMarkerFlags(IsFileEntry, IsFileExit, FileKind, *this))
       return;
+    if (!SourceMgr.isWrittenInBuiltinFile(DigitTok.getLocation()) &&
+        !SourceMgr.isWrittenInCommandLineFile(DigitTok.getLocation()))
+      Diag(StrTok, diag::ext_pp_gnu_line_directive);
 
     // Exiting to an empty string means pop to the including file, so leave
     // FilenameID as -1 in that case.
     if (!(IsFileExit && Literal.GetString().empty()))
       FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
   }
 
   // Create a line note with this information.

clang/test/Preprocessor/line-directive-system-headers.c

+// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -fsyntax-only -pedantic -verify=system -Wsystem-headers %s
+
+int x;
+
+# 42 // #1
+// expected-warning@#1 {{this style of line directive is a GNU extension}}
+// system-warning@#1 {{this style of line directive is a GNU extension}}
+# 42 "foo" // #2
+// expected-warning@#2 {{this style of line directive is a GNU extension}}
+// system-warning@#2 {{this style of line directive is a GNU extension}}
+# 42 "foo" 2 // #3
+// expected-error@#3 {{invalid line marker flag '2': cannot pop empty include stack}}
+// system-error@#3 {{invalid line marker flag '2': cannot pop empty include stack}}
+# 42 "foo" 1 3  // #4: enter
+// Warnings silenced when -Wsystem-headers isn't passed.
+// system-warning@#4 {{this style of line directive is a GNU extension}}
+# 42 "foo" 2 3  // #5: exit
+// Warnings silenced when -Wsystem-headers isn't passed.
+// system-warning@#5 {{this style of line directive is a GNU extension}}
+# 42 "foo" 2 3 4 // #6
+// expected-error@#6 {{invalid line marker flag '2': cannot pop empty include stack}}
+// system-error@#6 {{invalid line marker flag '2': cannot pop empty include stack}}
+# 42 "foo" 3 4 // #7
+// expected-warning@#7 {{this style of line directive is a GNU extension}}
+// system-warning@#7 {{this style of line directive is a GNU extension}}
+
+
+// Verify that linemarker diddling of the system header flag works.
+
+# 192 "glomp.h" // #8: not a system header.
+// expected-warning@#8 {{this style of line directive is a GNU extension}}
+// system-warning@#8 {{this style of line directive is a GNU extension}}
+
+# 192 "glomp.h" 3 // #9: System header.
+// Warnings silenced when -Wsystem-headers isn't passed.
+// system-warning@#9 {{this style of line directive is a GNU extension}}
+
+#line 42 "blonk.h"  // doesn't change system headerness.
+
+# 97     // #10: doesn't change system headerness.
+// Warnings silenced when -Wsystem-headers isn't passed.
+// system-warning@#10 {{this style of line directive is a GNU extension}}
+
+# 42 "blonk.h"  // #11: DOES change system headerness.
+// Warnings silenced when -Wsystem-headers isn't passed.
+// system-warning@#11 {{this style of line directive is a GNU extension}}

clang/test/Preprocessor/line-directive.c

 // RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -pedantic %s
 // RUN: not %clang_cc1 -E %s 2>&1 | grep 'blonk.c:92:2: error: ABC'
 // RUN: not %clang_cc1 -E %s 2>&1 | grep 'blonk.c:93:2: error: DEF'
 // RUN: not %clang_cc1 -E %s 2>&1 | grep 'line-directive.c:11:2: error: MAIN7'
 // RUN: not %clang_cc1 -E %s 2>&1 | grep 'enter-1:118:2: error: ENTER1'
 // RUN: not %clang_cc1 -E %s 2>&1 | grep 'main:121:2: error: MAIN2'
 
 // expected-error@+1{{cannot pop empty include stack}}
 # 20 "" 2
 
 // a push/pop before any other line control
-# 10 "enter-0" 1
-# 11 "" 2 // pop to main file
+# 10 "enter-0" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 11 "" 2 // pop to main file: expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN7
 // expected-error@-1{{MAIN7}}
 
 #line 'a'            // expected-error {{#line directive requires a positive integer argument}}
 #line 0              // expected-warning {{#line directive with zero argument is a GNU extension}}
 #line 00             // expected-warning {{#line directive with zero argument is a GNU extension}}
 #line 2147483648     // expected-warning {{C requires #line number to be less than 2147483648, allowed as extension}}
 #line 42             // ok
 #line 42 'a'         // expected-error {{invalid filename for #line directive}}
 #line 42 "foo/bar/baz.h"  // ok
 
 
 // #line directives expand macros.
 #define A 42 "foo"
 #line A
 
-# 42
-# 42 "foo"
+# 42 // expected-warning {{this style of line directive is a GNU extension}}
+# 42 "foo" // expected-warning {{this style of line directive is a GNU extension}}
 # 42 "foo" 2 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
+// The next two lines do not get diagnosed because they are considered to be
+// within the system header, where diagnostics are suppressed.
 # 42 "foo" 1 3  // enter
 # 42 "foo" 2 3  // exit
 # 42 "foo" 2 3 4 // expected-error {{invalid line marker flag '2': cannot pop empty include stack}}
-# 42 "foo" 3 4
+# 42 "foo" 3 4 // expected-warning {{this style of line directive is a GNU extension}}
 
 # 'a'            // expected-error {{invalid preprocessing directive}}
 # 42 'f'         // expected-error {{invalid filename for line marker directive}}
 # 42 1 3         // expected-error {{invalid filename for line marker directive}}
 # 42 "foo" 3 1   // expected-error {{invalid flag line marker directive}}
 # 42 "foo" 42    // expected-error {{invalid flag line marker directive}}
 # 42 "foo" 1 2   // expected-error {{invalid flag line marker directive}}
 # 42a33          // expected-error {{GNU line marker directive requires a simple digit sequence}}
 
 // These are checked by the RUN line.
 #line 92 "blonk.c"
 #error ABC
 #error DEF
 // expected-error@-2 {{ABC}}
 #line 150
 // expected-error@-3 {{DEF}}
 
 
 // Verify that linemarker diddling of the system header flag works.
 
-# 192 "glomp.h" // not a system header.
+# 192 "glomp.h" // not a system header.: expected-warning {{this style of line directive is a GNU extension}}
 typedef int x;  // expected-note {{previous definition is here}}
 typedef int x;  // expected-warning {{redefinition of typedef 'x' is a C11 feature}}
 
 # 192 "glomp.h" 3 // System header.
 typedef int y;  // ok
 typedef int y;  // ok
 
 typedef int q;  // q is in system header.
 
 #line 42 "blonk.h"  // doesn't change system headerness.
 
 typedef int z;  // ok
 typedef int z;  // ok
 
 # 97     // doesn't change system headerness.
 
 typedef int z1;  // ok
 typedef int z1;  // ok
 
 # 42 "blonk.h"  // DOES change system headerness.
 
 typedef int w;  // expected-note {{previous definition is here}}
 typedef int w;  // expected-warning {{redefinition of typedef 'w' is a C11 feature}}
 
 typedef int q;  // original definition in system header, should not diagnose.
 
 // This should not produce an "extra tokens at end of #line directive" warning,
 // because #line is allowed to contain expanded tokens.
 #define EMPTY()
 #line 2 "foo.c" EMPTY( )
 #line 2 "foo.c" NONEMPTY( )  // expected-warning{{extra tokens at end of #line directive}}
 
 // PR3940
 #line 0xf  // expected-error {{#line directive requires a simple digit sequence}}
 #line 42U  // expected-error {{#line directive requires a simple digit sequence}}
 
 
 // Line markers are digit strings interpreted as decimal numbers, this is
 // 10, not 8.
 #line 010  // expected-warning {{#line directive interprets number as decimal, not octal}}
 extern int array[__LINE__ == 10 ? 1:-1];
 
-# 020      // expected-warning {{GNU line marker directive interprets number as decimal, not octal}}
+# 020      // expected-warning {{GNU line marker directive interprets number as decimal, not octal}} expected-warning {{this style of line directive is a GNU extension}}
 extern int array_gnuline[__LINE__ == 20 ? 1:-1];
 
 /* PR3917 */
 #line 41
 extern char array2[\
 _\
 _LINE__ == 42 ? 1: -1];  /* line marker is location of first _ */
 
-# 51
+# 51 // expected-warning {{this style of line directive is a GNU extension}}
 extern char array2_gnuline[\
 _\
 _LINE__ == 52 ? 1: -1];  /* line marker is location of first _ */
 
 // rdar://11550996
 #line 0 "line-directive.c" // expected-warning {{#line directive with zero argument is a GNU extension}}
 undefined t; // expected-error {{unknown type name 'undefined'}}
 
-# 115 "main"
-# 116 "enter-1" 1
-# 117 "enter-2" 1
-# 118 "" 2 // pop to enter-1
+# 115 "main" // expected-warning {{this style of line directive is a GNU extension}}
+# 116 "enter-1" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 117 "enter-2" 1 // expected-warning {{this style of line directive is a GNU extension}}
+# 118 "" 2 // pop to enter-1: expected-warning {{this style of line directive is a GNU extension}}
 #error ENTER1
 // expected-error@-1{{ENTER1}}
-# 121 "" 2 // pop to "main"
+# 121 "" 2 // pop to "main": expected-warning {{this style of line directive is a GNU extension}}
 #error MAIN2
 // expected-error@-1{{MAIN2}}

clang/test/SemaCXX/constexpr-string.cpp

 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
-# 9 "/usr/include/string.h" 1 3 4
+# 9 "/usr/include/string.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
   typedef decltype(sizeof(int)) size_t;
 
   extern size_t strlen(const char *p);
 
   extern int strcmp(const char *s1, const char *s2);
   extern int strncmp(const char *s1, const char *s2, size_t n);
   extern int memcmp(const void *s1, const void *s2, size_t n);
 
 #ifdef GNUMODE
   extern int bcmp(const void *s1, const void *s2, size_t n);
 #endif
 
   extern char *strchr(const char *s, int c);
   extern void *memchr(const void *s, int c, size_t n);
 
   extern void *memcpy(void *d, const void *s, size_t n);
   extern void *memmove(void *d, const void *s, size_t n);
 }
 # 25 "SemaCXX/constexpr-string.cpp" 2
 
-# 27 "/usr/include/wchar.h" 1 3 4
+# 27 "/usr/include/wchar.h" 1 3 4  // expected-warning {{this style of line directive is a GNU extension}}
 extern "C" {
 #if NO_PREDEFINED_WCHAR_T
   typedef decltype(L'0') wchar_t;
 #endif
   extern size_t wcslen(const wchar_t *p);
 
   extern int wcscmp(const wchar_t *s1, const wchar_t *s2);
   extern int wcsncmp(const wchar_t *s1, const wchar_t *s2, size_t n);