[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

Wild guess is that `2> %t.err` should be removed from the `-verify` lines? That 
change passes in the single env I have access to.  @rnk any idea what might be 
going on?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72405/new/

https://reviews.llvm.org/D72405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

Hi Zach,

The tests in this patch are failing on this PS4 windows bot. Could you take a 
look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/30273/steps/test-check-all/logs/stdio

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72405/new/

https://reviews.llvm.org/D72405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f9cf42facaf: Allow /D flags absent during PCH creation 
under msvc-compat (authored by zahen, committed by rnk).

Changed prior to commit:
  https://reviews.llvm.org/D72405?vs=238147=238149#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72405/new/

https://reviews.llvm.org/D72405

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/PCH/ms-pch-macro.c


Index: clang/test/PCH/ms-pch-macro.c
===
--- /dev/null
+++ clang/test/PCH/ms-pch-macro.c
@@ -0,0 +1,39 @@
+// Test -D and -U interaction with a PCH when -fms-extensions is enabled.
+
+// RUN: %clang_cc1 -DFOO %S/variables.h -emit-pch -o %t1.pch
+
+// RUN: not %clang_cc1 -DFOO=blah -DBAR=int -include-pch %t1.pch 
-pch-through-header=%S/variables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err
+
+// RUN: not %clang_cc1 -UFOO -DBAR=int -include-pch %t1.pch %s 
-pch-through-header=%S/variables.h 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err
+
+// RUN: %clang_cc1 -include-pch %t1.pch -DBAR=int 
-pch-through-header=%S/variables.h -verify %s 2> %t.err
+
+// Enabling MS extensions should allow us to add BAR definitions.
+// RUN: %clang_cc1 -fms-extensions -DFOO %S/variables.h -emit-pch -o %t1.pch
+// RUN: %clang_cc1 -fms-extensions -include-pch %t1.pch -DBAR=int 
-pch-through-header=%S/variables.h -verify %s 2> %t.err
+
+#include "variables.h"
+
+BAR bar = 17;
+#ifndef _MSC_EXTENSIONS
+// expected-error@-2 {{unknown type name 'BAR'}}
+#endif
+
+#ifndef FOO
+#  error FOO was not defined
+#endif
+
+#if FOO != 1
+#  error FOO has the wrong definition
+#endif
+
+#if defined(_MSC_EXTENSIONS) && !defined(BAR)
+#  error BAR was not defined
+#endif
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header 
('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd 
on the command line
+
+// expected-warning@1 {{definition of macro 'BAR' does not match definition in 
precompiled header}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2727,7 +2727,9 @@
  /*Syntactic=*/LangOpts.MicrosoftExt))
   Diag(MI->getDefinitionLoc(), diag::warn_pp_macro_def_mismatch_with_pch)
   << MacroNameTok.getIdentifierInfo();
-return;
+// Issue the diagnostic but allow the change if msvc extensions are enabled
+if (!LangOpts.MicrosoftExt)
+  return;
   }
 
   // Finally, if this identifier already had a macro defined for it, verify 
that


Index: clang/test/PCH/ms-pch-macro.c
===
--- /dev/null
+++ clang/test/PCH/ms-pch-macro.c
@@ -0,0 +1,39 @@
+// Test -D and -U interaction with a PCH when -fms-extensions is enabled.
+
+// RUN: %clang_cc1 -DFOO %S/variables.h -emit-pch -o %t1.pch
+
+// RUN: not %clang_cc1 -DFOO=blah -DBAR=int -include-pch %t1.pch -pch-through-header=%S/variables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err
+
+// RUN: not %clang_cc1 -UFOO -DBAR=int -include-pch %t1.pch %s -pch-through-header=%S/variables.h 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err
+
+// RUN: %clang_cc1 -include-pch %t1.pch -DBAR=int -pch-through-header=%S/variables.h -verify %s 2> %t.err
+
+// Enabling MS extensions should allow us to add BAR definitions.
+// RUN: %clang_cc1 -fms-extensions -DFOO %S/variables.h -emit-pch -o %t1.pch
+// RUN: %clang_cc1 -fms-extensions -include-pch %t1.pch -DBAR=int -pch-through-header=%S/variables.h -verify %s 2> %t.err
+
+#include "variables.h"
+
+BAR bar = 17;
+#ifndef _MSC_EXTENSIONS
+// expected-error@-2 {{unknown type name 'BAR'}}
+#endif
+
+#ifndef FOO
+#  error FOO was not defined
+#endif
+
+#if FOO != 1
+#  error FOO has the wrong definition
+#endif
+
+#if defined(_MSC_EXTENSIONS) && !defined(BAR)
+#  error BAR was not defined
+#endif
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header ('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd on the command line
+
+// expected-warning@1 {{definition of macro 'BAR' does not match definition in precompiled header}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2727,7 +2727,9 @@
  /*Syntactic=*/LangOpts.MicrosoftExt))
   Diag(MI->getDefinitionLoc(), diag::warn_pp_macro_def_mismatch_with_pch)
   << MacroNameTok.getIdentifierInfo();
-return;
+// Issue the diagnostic but allow the change if msvc extensions are enabled
+if 

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

In D72405#1818237 , @zahen wrote:

> I had no luck converting the CHECK-FOO & CHECK-NOFOO tests to use `-verify` 
> because the errors were reported against "(frontend)"


I converted what I could over to `-verify` and switched to testing clang -cc1.

I think this looks good, I'm going to land it, since that's what I've done for 
you in the past. We have git now, it'll be attributed properly and all that. :)

Let me know if you get commit access in the future to push your own patches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72405/new/

https://reviews.llvm.org/D72405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 238147.
rnk added a comment.

- use %clang_cc1 as possible in tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72405/new/

https://reviews.llvm.org/D72405

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/PCH/ms-pch-macro.c


Index: clang/test/PCH/ms-pch-macro.c
===
--- /dev/null
+++ clang/test/PCH/ms-pch-macro.c
@@ -0,0 +1,39 @@
+// Test -D and -U interaction with a PCH when -fms-extensions is enabled.
+
+// RUN: %clang_cc1 -DFOO %S/variables.h -emit-pch -o %t1.pch
+
+// RUN: not %clang_cc1 -DFOO=blah -DBAR=int -include-pch %t1.pch 
-pch-through-header=%S/variables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err
+
+// RUN: not %clang_cc1 -UFOO -DBAR=int -include-pch %t1.pch %s 
-pch-through-header=%S/variables.h 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err
+
+// RUN: %clang_cc1 -include-pch %t1.pch -DBAR=int 
-pch-through-header=%S/variables.h -verify %s 2> %t.err
+
+// Enabling MS extensions should allow us to add BAR definitions.
+// RUN: %clang_cc1 -fms-extensions -DFOO %S/variables.h -emit-pch -o %t1.pch
+// RUN: %clang_cc1 -fms-extensions -include-pch %t1.pch -DBAR=int 
-pch-through-header=%S/variables.h -verify %s 2> %t.err
+
+#include "variables.h"
+
+BAR bar = 17;
+#ifndef _MSC_EXTENSIONS
+// expected-error@-2 {{unknown type name 'BAR'}}
+#endif
+
+#ifndef FOO
+#  error FOO was not defined
+#endif
+
+#if FOO != 1
+#  error FOO has the wrong definition
+#endif
+
+#if !defined(_MSC_EXTENSIONS) && !defined(BAR)
+#  error BAR was not defined
+#endif
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header 
('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd 
on the command line
+
+// expected-warning@1 {{definition of macro 'BAR' does not match definition in 
precompiled header}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2727,7 +2727,9 @@
  /*Syntactic=*/LangOpts.MicrosoftExt))
   Diag(MI->getDefinitionLoc(), diag::warn_pp_macro_def_mismatch_with_pch)
   << MacroNameTok.getIdentifierInfo();
-return;
+// Issue the diagnostic but allow the change if msvc extensions are enabled
+if (!LangOpts.MicrosoftExt)
+  return;
   }
 
   // Finally, if this identifier already had a macro defined for it, verify 
that


Index: clang/test/PCH/ms-pch-macro.c
===
--- /dev/null
+++ clang/test/PCH/ms-pch-macro.c
@@ -0,0 +1,39 @@
+// Test -D and -U interaction with a PCH when -fms-extensions is enabled.
+
+// RUN: %clang_cc1 -DFOO %S/variables.h -emit-pch -o %t1.pch
+
+// RUN: not %clang_cc1 -DFOO=blah -DBAR=int -include-pch %t1.pch -pch-through-header=%S/variables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err
+
+// RUN: not %clang_cc1 -UFOO -DBAR=int -include-pch %t1.pch %s -pch-through-header=%S/variables.h 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err
+
+// RUN: %clang_cc1 -include-pch %t1.pch -DBAR=int -pch-through-header=%S/variables.h -verify %s 2> %t.err
+
+// Enabling MS extensions should allow us to add BAR definitions.
+// RUN: %clang_cc1 -fms-extensions -DFOO %S/variables.h -emit-pch -o %t1.pch
+// RUN: %clang_cc1 -fms-extensions -include-pch %t1.pch -DBAR=int -pch-through-header=%S/variables.h -verify %s 2> %t.err
+
+#include "variables.h"
+
+BAR bar = 17;
+#ifndef _MSC_EXTENSIONS
+// expected-error@-2 {{unknown type name 'BAR'}}
+#endif
+
+#ifndef FOO
+#  error FOO was not defined
+#endif
+
+#if FOO != 1
+#  error FOO has the wrong definition
+#endif
+
+#if !defined(_MSC_EXTENSIONS) && !defined(BAR)
+#  error BAR was not defined
+#endif
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header ('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd on the command line
+
+// expected-warning@1 {{definition of macro 'BAR' does not match definition in precompiled header}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2727,7 +2727,9 @@
  /*Syntactic=*/LangOpts.MicrosoftExt))
   Diag(MI->getDefinitionLoc(), diag::warn_pp_macro_def_mismatch_with_pch)
   << MacroNameTok.getIdentifierInfo();
-return;
+// Issue the diagnostic but allow the change if msvc extensions are enabled
+if (!LangOpts.MicrosoftExt)
+  return;
   }
 
   // Finally, if this identifier already had a macro defined for it, verify that
___
cfe-commits mailing 

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Zachary Henkel via Phabricator via cfe-commits]

zahen updated this revision to Diff 237795.
zahen added a comment.

Incorporate review feedback.

I had no luck converting the CHECK-FOO & CHECK-NOFOO tests to use `-verify` 
because the errors were reported against "(frontend)"

  error: 'error' diagnostics seen but not expected:
(frontend): definition of macro 'FOO' differs between the precompiled 
header ('1') and the command line ('blah')


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72405/new/

https://reviews.llvm.org/D72405

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/PCH/fuzzy-pch-msvc.c


Index: clang/test/PCH/fuzzy-pch-msvc.c
===
--- /dev/null
+++ clang/test/PCH/fuzzy-pch-msvc.c
@@ -0,0 +1,32 @@
+// Test -D and -U interaction with a PCH when running clang-cl.
+
+// RUN: %clang_cl -DFOO -Yc %S/variables.h
+
+// RUN: not %clang_cl -c -DFOO=blah -DBAR=int -Wno-microsoft-include 
-Yuvariables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err
+
+// RUN: not %clang_cl -c -UFOO -DBAR=int -Wno-microsoft-include -Yuvariables.h 
%s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err 
+
+// RUN: %clang_cl -c -DBAR=int -Wno-microsoft-include -Yuvariables.h -Xclang 
-verify %s 
+#include "variables.h"
+
+BAR bar = 17;
+
+#ifndef FOO
+#  error FOO was not defined
+#endif
+
+#if FOO != 1
+#  error FOO has the wrong definition
+#endif
+
+#ifndef BAR
+#  error BAR was not defined 
+#endif
+
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header 
('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd 
on the command line
+
+// expected-warning@1 {{definition of macro 'BAR' does not match definition in 
precompiled header}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2727,7 +2727,9 @@
  /*Syntactic=*/LangOpts.MicrosoftExt))
   Diag(MI->getDefinitionLoc(), diag::warn_pp_macro_def_mismatch_with_pch)
   << MacroNameTok.getIdentifierInfo();
-return;
+// Issue the diagnostic but allow the change if msvc extensions are enabled
+if (!LangOpts.MicrosoftExt)
+  return;
   }
 
   // Finally, if this identifier already had a macro defined for it, verify 
that


Index: clang/test/PCH/fuzzy-pch-msvc.c
===
--- /dev/null
+++ clang/test/PCH/fuzzy-pch-msvc.c
@@ -0,0 +1,32 @@
+// Test -D and -U interaction with a PCH when running clang-cl.
+
+// RUN: %clang_cl -DFOO -Yc %S/variables.h
+
+// RUN: not %clang_cl -c -DFOO=blah -DBAR=int -Wno-microsoft-include -Yuvariables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err
+
+// RUN: not %clang_cl -c -UFOO -DBAR=int -Wno-microsoft-include -Yuvariables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err 
+
+// RUN: %clang_cl -c -DBAR=int -Wno-microsoft-include -Yuvariables.h -Xclang -verify %s 
+#include "variables.h"
+
+BAR bar = 17;
+
+#ifndef FOO
+#  error FOO was not defined
+#endif
+
+#if FOO != 1
+#  error FOO has the wrong definition
+#endif
+
+#ifndef BAR
+#  error BAR was not defined 
+#endif
+
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header ('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd on the command line
+
+// expected-warning@1 {{definition of macro 'BAR' does not match definition in precompiled header}}
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2727,7 +2727,9 @@
  /*Syntactic=*/LangOpts.MicrosoftExt))
   Diag(MI->getDefinitionLoc(), diag::warn_pp_macro_def_mismatch_with_pch)
   << MacroNameTok.getIdentifierInfo();
-return;
+// Issue the diagnostic but allow the change if msvc extensions are enabled
+if (!LangOpts.MicrosoftExt)
+  return;
   }
 
   // Finally, if this identifier already had a macro defined for it, verify that
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Reid Kleckner via Phabricator via cfe-commits]

rnk added subscribers: aganea, mikerice.
rnk added a comment.

Honestly, MSVC's behavior makes more sense to me. Usually warnings tell the 
user they are doing something silly, and then let them do it anyway. Without 
this change, there is no way to add extra macros to some compilations, even if 
they won't affect the already-parsed PCH blob.

+@aganea, since I think he has used /Yc /Yu, and @mikerice, the last person to 
touch it.

BTW, this raises the issue of how PCH will work with dllexport, which typically 
works by defining some kind of "FOO_DLL" macro to indicate that some 
annotations should expand to `__declspec(dllexport)`. I suppose this change 
won't solve that problem, but it's still interesting.




Comment at: clang/lib/Lex/PPDirectives.cpp:2728
  /*Syntactic=*/LangOpts.MicrosoftExt))
   Diag(MI->getDefinitionLoc(), diag::warn_pp_macro_def_mismatch_with_pch)
   << MacroNameTok.getIdentifierInfo();

I think the case of passing -D to compilation but not PCH is likely to be the 
overwhelming majority of cases that users run into, and MSVC has a nice, 
specific diagnostic for that case (probably "!OtherMI"). We should do that too, 
but do not feel obligated to do that in this change.



Comment at: clang/lib/Lex/PPDirectives.cpp:2731
+// Issue the diagnostic but allow the change under msvc compatability mode
+if (!getLangOpts().MSVCCompat)
+  return;

PCH is in the space of implementation-defined behaviors, not standards-mandated 
behaviors. I think MicrosoftExt (-fms-extensions) would be a better condition 
here. Besides, it's consistent with the use just above.



Comment at: clang/test/PCH/fuzzy-pch-msvc.c:30-31
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header 
('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd 
on the command line
+// CHECK-BAR: error: definition of macro 'BAR' does not match definition in 
precompiled header [-Werror,-Wclang-cl-pch]

For this stuff, it would be nicer to use `%clang_cc1 -verify` instead of 
FileCheck. You can do things like:
```
// expected-warning@2 {{'FOO' macro redefined}}
// expected-note@1 {{previous definition is here}}
int main() {}
```
I just checked, and that works.

I see that the fuzzy-pch.c test that you based this on probably predates clang 
-cc1 -verify.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72405/new/

https://reviews.llvm.org/D72405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

My change keeps the diagnostic so consumers can opt into the same enforcement 
that exists today.  Furthermore, the existing fuzzy-pch.c tests show that new 
-D flags are allowed under a "clangier" PCH structure.  None of the existing 
tests error on:

  BAR bar = 17;

when `-DBar=int` is only part of the the test file's command line.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72405/new/

https://reviews.llvm.org/D72405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Why do you want this? Isn't it always easy to fix your build instead?

In general, stricter is better when we can get away with it, and since we've 
had this behavior for a while…


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72405/new/

https://reviews.llvm.org/D72405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Zachary Henkel via Phabricator via cfe-commits]

zahen created this revision.
zahen added reviewers: rnk, thakis, hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Before this patch adding a new /D flag when compiling a source file that 
consumed a PCH with clang-cl would issue a diagnostic and then fail.  With the 
patch, the diagnostic is still issued but the definition is accepted.  This 
matches the msvc behavior.  The fuzzy-pch-msvc.c is a clone of the existing 
fuzzy-pch.c tests with some msvc specific rework.

msvc diagnostic:

  warning C4605: '/DBAR=int' specified on current command line, but was not 
specified when precompiled header was built

Output of the CHECK-BAR test prior to the code change:

  (1,9): warning: definition of macro 'BAR' does not match definition 
in precompiled header [-Wclang-cl-pch]
  #define BAR int
  ^
  D:\repos\llvm\llvm-project\clang\test\PCH\fuzzy-pch-msvc.c(12,1): error: 
unknown type name 'BAR'
  BAR bar = 17;
  ^
  D:\repos\llvm\llvm-project\clang\test\PCH\fuzzy-pch-msvc.c(23,4): error: BAR 
was not defined
  #  error BAR was not defined
 ^
  1 warning and 2 errors generated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72405

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/PCH/fuzzy-pch-msvc.c


Index: clang/test/PCH/fuzzy-pch-msvc.c
===
--- /dev/null
+++ clang/test/PCH/fuzzy-pch-msvc.c
@@ -0,0 +1,32 @@
+// Test -D and -U interaction with a PCH when running clang-cl.
+
+// RUN: %clang_cl -DFOO -Yc %S/variables.h
+// RUN: %clang_cl -c -DBAR=int -Wno-microsoft-include -Yuvariables.h %s
+// RUN: %clang_cl -c -DFOO -DBAR=int -Wno-microsoft-include -Yuvariables.h %s
+// RUN: not %clang_cl -c -DFOO=blah -DBAR=int -Wno-microsoft-include 
-Yuvariables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err
+// RUN: not %clang_cl -c -UFOO -DBAR=int -Wno-microsoft-include -Yuvariables.h 
%s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err
+
+// RUN: not %clang_cl -c -DBAR=int -WX -Wno-microsoft-include -Yuvariables.h 
%s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-BAR %s < %t.err
+
+#include "variables.h"
+
+BAR bar = 17;
+
+#ifndef FOO
+#  error FOO was not defined
+#endif
+
+#if FOO != 1
+#  error FOO has the wrong definition
+#endif
+
+#ifndef BAR
+#  error BAR was not defined
+#endif
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header 
('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd 
on the command line
+// CHECK-BAR: error: definition of macro 'BAR' does not match definition in 
precompiled header [-Werror,-Wclang-cl-pch]
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2727,7 +2727,9 @@
  /*Syntactic=*/LangOpts.MicrosoftExt))
   Diag(MI->getDefinitionLoc(), diag::warn_pp_macro_def_mismatch_with_pch)
   << MacroNameTok.getIdentifierInfo();
-return;
+// Issue the diagnostic but allow the change under msvc compatability mode
+if (!getLangOpts().MSVCCompat)
+  return;
   }
 
   // Finally, if this identifier already had a macro defined for it, verify 
that


Index: clang/test/PCH/fuzzy-pch-msvc.c
===
--- /dev/null
+++ clang/test/PCH/fuzzy-pch-msvc.c
@@ -0,0 +1,32 @@
+// Test -D and -U interaction with a PCH when running clang-cl.
+
+// RUN: %clang_cl -DFOO -Yc %S/variables.h
+// RUN: %clang_cl -c -DBAR=int -Wno-microsoft-include -Yuvariables.h %s
+// RUN: %clang_cl -c -DFOO -DBAR=int -Wno-microsoft-include -Yuvariables.h %s
+// RUN: not %clang_cl -c -DFOO=blah -DBAR=int -Wno-microsoft-include -Yuvariables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-FOO %s < %t.err
+// RUN: not %clang_cl -c -UFOO -DBAR=int -Wno-microsoft-include -Yuvariables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-NOFOO %s < %t.err
+
+// RUN: not %clang_cl -c -DBAR=int -WX -Wno-microsoft-include -Yuvariables.h %s 2> %t.err
+// RUN: FileCheck -check-prefix=CHECK-BAR %s < %t.err
+
+#include "variables.h"
+
+BAR bar = 17;
+
+#ifndef FOO
+#  error FOO was not defined
+#endif
+
+#if FOO != 1
+#  error FOO has the wrong definition
+#endif
+
+#ifndef BAR
+#  error BAR was not defined
+#endif
+
+// CHECK-FOO: definition of macro 'FOO' differs between the precompiled header ('1') and the command line ('blah')
+// CHECK-NOFOO: macro 'FOO' was defined in the precompiled header but undef'd on the command line
+// CHECK-BAR: error: definition of macro 'BAR' does not match definition in precompiled header [-Werror,-Wclang-cl-pch]
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2727,7 +2727,9 @@