[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-06-01 Thread Carlos Eduardo Seo via Phabricator via cfe-commits 
cseo added a comment.

In D144654#4381893 , @nickdesaulniers 
wrote:

> In D144654#4380488 , @john.brawn 
> wrote:
>
>> gcc has the same warning so I wasn't expecting this cause to change 
>> problems, but looking more closely at gcc's behaviour it looks like it only 
>> warns for some builtin macros and not others (though glancing over the gcc 
>> source code it's not clear which macros and for what reason).
>>
>> I'll look at this some more and see if I can improve the behaviour.
>
> Based on
> https://lore.kernel.org/llvm/6475a837.170a0220.77d4a.1...@mx.google.com/T/#u
> I think the following macros aren't warned on: https://godbolt.org/z/dfqnG7bae
>
>   #undef __INT32_TYPE__
>   #undef __UINT32_TYPE__
>   #undef __UINTPTR_TYPE__
>   #undef __i386__
>   #undef __UINT64_TYPE__
>   #undef __INT64_TYPE__

glibc build also fails because of

#undef  __OPTIMIZE__


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-30 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added a comment.

In D144654#4380488 , @john.brawn 
wrote:

> gcc has the same warning so I wasn't expecting this cause to change problems, 
> but looking more closely at gcc's behaviour it looks like it only warns for 
> some builtin macros and not others (though glancing over the gcc source code 
> it's not clear which macros and for what reason).
>
> I'll look at this some more and see if I can improve the behaviour.

Based on
https://lore.kernel.org/llvm/6475a837.170a0220.77d4a.1...@mx.google.com/T/#u
I think the following macros aren't warned on: https://godbolt.org/z/dfqnG7bae

  #undef __INT32_TYPE__
  #undef __UINT32_TYPE__
  #undef __UINTPTR_TYPE__
  #undef __i386__
  #undef __UINT64_TYPE__
  #undef __INT64_TYPE__


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-30 Thread Martin Storsjö via Phabricator via cfe-commits 
mstorsjo added a comment.

FWIW, I've also run into noisy warnings caused by this, in a few places. 
D151662  fixes a bunch of clang's tests when 
run in MinGW configurations, and 
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230526104837.20594-1-mar...@martin.st/
 tries to avoid warnings due to `-U__STRICT_ANSI__` in an external project.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-30 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added a comment.

gcc has the same warning so I wasn't expecting this cause to change problems, 
but looking more closely at gcc's behaviour it looks like it only warns for 
some builtin macros and not others (though glancing over the gcc source code 
it's not clear which macros and for what reason).

I'll look at this some more and see if I can improve the behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-26 Thread Nathan Chancellor via Phabricator via cfe-commits 
nathanchance added subscribers: nickdesaulniers, nathanchance.
nathanchance added a comment.

For what it's worth, this change is quite noisy for the Linux kernel, as there 
are certain macros undefined in header files that are included in many other 
headers. Additionally, at least one new instance is in a UAPI header, so it 
cannot be changed to my knowledge (honestly, all of these appear to be 
intentional, so I am not sure any of them should be changed). Is there any way 
for us to opt out of this? It would be a shame if we had to outright disable 
`-Wbuiltin-macro-redefined`, it has caught issues before (the kernel is 
generally against opting out on a per-location basis but does have the 
infrastructure to do so, not sure if it will be available for all these 
locations though...).

`arch/arm/include/uapi/asm/types.h`: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/include/uapi/asm/types.h?h=v6.4-rc3

  In file included from scripts/mod/devicetable-offsets.c:3:
  In file included from include/linux/mod_devicetable.h:12:
  In file included from include/uapi/linux/mei.h:10:
  In file included from include/uapi/linux/mei_uuid.h:12:
  In file included from include/linux/types.h:6:
  In file included from include/uapi/linux/types.h:5:
  arch/arm/include/uapi/asm/types.h:27:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef __INT32_TYPE__
 ^
  arch/arm/include/uapi/asm/types.h:32:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef __UINT32_TYPE__
 ^
  arch/arm/include/uapi/asm/types.h:37:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef __UINTPTR_TYPE__
 ^
  3 errors generated.

`arch/arm64/include/asm/neon-intrinsics.h`: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/include/asm/neon-intrinsics.h?h=v6.4-rc3

  In file included from arch/arm64/lib/xor-neon.c:11:
  arch/arm64/include/asm/neon-intrinsics.h:18:8: error: undefining builtin 
macro [-Werror,-Wbuiltin-macro-redefined]
  #undef __INT64_TYPE__
 ^
  arch/arm64/include/asm/neon-intrinsics.h:23:8: error: undefining builtin 
macro [-Werror,-Wbuiltin-macro-redefined]
  #undef __UINT64_TYPE__
 ^
  2 errors generated.

`include/linux/drbd_genl_api.h` (this may be unnecessary now?): 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/drbd_genl_api.h?h=v6.4-rc3

  In file included from drivers/block/drbd/drbd_debugfs.c:11:
  In file included from drivers/block/drbd/drbd_int.h:35:
  include/linux/drbd_genl_api.h:47:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef linux
 ^
  1 error generated.

`arch/mips/kernel/vmlinux.lds.S` (there is little context available as to why 
this exists): 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/kernel/vmlinux.lds.S?h=v6.4-rc3

  In file included from :357:
  :6:8: error: undefining builtin macro 
[-Werror,-Wbuiltin-macro-redefined]
  #undef mips
 ^
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-25 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added a comment.

D150966  should fix the failure that was seen 
in buildbots, so I've now re-committed this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-17 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Reverted in cf8c358dc9414e3de9f65eebdfdcb66dd5817857 for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-17 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

In D144654#4349908 , @john.brawn 
wrote:

> This patch caused a failure in AArch64 buildbots due to 
> AArch64TargetInfo::getTargetDefines redefining _LP64 and __LP64__. Fixed in 
> https://reviews.llvm.org/rGe55d52cd34fb7a6a6617639d147b9d0abaceeeab.

Tests are still failing even with this: 
http://45.33.8.238/macm1/60903/step_7.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-17 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added a comment.

This patch caused a failure in AArch64 buildbots due to 
AArch64TargetInfo::getTargetDefines redefining _LP64 and __LP64__. Fixed in 
https://reviews.llvm.org/rGe55d52cd34fb7a6a6617639d147b9d0abaceeeab.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-17 Thread John Brawn via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG22e3f587fd1f: [Lex] Warn when defining or undefining any 
builtin macro (authored by john.brawn).

Changed prior to commit:
  https://reviews.llvm.org/D144654?vs=522683=522989#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/builtin_redef.c
  clang/test/Preprocessor/macro-reserved.c
  clang/test/Preprocessor/macro-reserved.cpp

Index: clang/test/Preprocessor/macro-reserved.cpp
===
--- clang/test/Preprocessor/macro-reserved.cpp
+++ clang/test/Preprocessor/macro-reserved.cpp
@@ -12,7 +12,7 @@
 #undef _HAVE_X
 #undef X__Y
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus
 
 // allowlisted definitions
Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -6,6 +6,7 @@
 #define __cplusplus
 #define _HAVE_X 0
 #define X__Y
+#define __STDC__ 1 // expected-warning {{redefining builtin macro}}
 
 #undef for
 #undef final
@@ -13,6 +14,7 @@
 #undef __cplusplus
 #undef _HAVE_X
 #undef X__Y
+#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
 
 // allowlisted definitions
 #define while while
Index: clang/test/Lexer/builtin_redef.c
===
--- clang/test/Lexer/builtin_redef.c
+++ clang/test/Lexer/builtin_redef.c
@@ -1,12 +1,24 @@
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
-// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
+// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
 
 // CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __TIME__ 1234
 // CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __DATE__
+// CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __STDC__ 1
+// CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __STDC_HOSTED__
 
 // CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __TIME__ 1234
+// CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __DATE__
+// CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __STDC__ 1
 // CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __STDC_HOSTED__
 
 int n = __TIME__;
 __DATE__
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -109,52 +109,52 @@
   PED_Elifndef
 };
 
+static bool isFeatureTestMacro(StringRef MacroName) {
+  // list from:
+  // * https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+  // * https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
+  // * man 7 feature_test_macros
+  // The list must be sorted for correct binary search.
+  static constexpr StringRef ReservedMacro[] = {
+  "_ATFILE_SOURCE",
+  "_BSD_SOURCE",
+  "_CRT_NONSTDC_NO_WARNINGS",
+  "_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
+  "_CRT_SECURE_NO_WARNINGS",
+  "_FILE_OFFSET_BITS",
+  "_FORTIFY_SOURCE",
+  "_GLIBCXX_ASSERTIONS",
+  "_GLIBCXX_CONCEPT_CHECKS",
+  "_GLIBCXX_DEBUG",
+  "_GLIBCXX_DEBUG_PEDANTIC",
+  "_GLIBCXX_PARALLEL",
+  "_GLIBCXX_PARALLEL_ASSERTIONS",
+  "_GLIBCXX_SANITIZE_VECTOR",
+  "_GLIBCXX_USE_CXX11_ABI",
+  "_GLIBCXX_USE_DEPRECATED",
+  "_GNU_SOURCE",
+  "_ISOC11_SOURCE",
+  "_ISOC95_SOURCE",
+  "_ISOC99_SOURCE",
+  "_LARGEFILE64_SOURCE",
+  "_POSIX_C_SOURCE",
+  "_REENTRANT",
+  "_SVID_SOURCE",
+  "_THREAD_SAFE",
+  "_XOPEN_SOURCE",
+  "_XOPEN_SOURCE_EXTENDED",
+  "__STDCPP_WANT_MATH_SPEC_FUNCS__",
+  "__STDC_FORMAT_MACROS",
+  };
+  return std::binary_search(std::begin(ReservedMacro), std::end(ReservedMacro),
+MacroName);
+}
+
 static MacroDiag

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-16 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, though precommit CI wasn't able to run on this, so please pay attention 
to the bots when you land in case there's fallout.




Comment at: clang/test/Preprocessor/macro-reserved.cpp:15
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus

john.brawn wrote:
> aaron.ballman wrote:
> > Why do we diagnose the undef but not the define?
> After the undef the builtin macro definition no longer exists, so when 
> Preprocessor::HandleDefineDirective checks for an existing definition to see 
> if it's a builtin it doesn't find one.
Ah, that makes sense. Thank you!


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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-16 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added inline comments.



Comment at: clang/test/Preprocessor/macro-reserved.cpp:15
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus

aaron.ballman wrote:
> Why do we diagnose the undef but not the define?
After the undef the builtin macro definition no longer exists, so when 
Preprocessor::HandleDefineDirective checks for an existing definition to see if 
it's a builtin it doesn't find one.


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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-16 Thread John Brawn via Phabricator via cfe-commits 
john.brawn updated this revision to Diff 522683.
john.brawn added a comment.

Adjusted in order to make clang-format happy.


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

https://reviews.llvm.org/D144654

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/builtin_redef.c
  clang/test/Preprocessor/macro-reserved.c
  clang/test/Preprocessor/macro-reserved.cpp

Index: clang/test/Preprocessor/macro-reserved.cpp
===
--- clang/test/Preprocessor/macro-reserved.cpp
+++ clang/test/Preprocessor/macro-reserved.cpp
@@ -12,7 +12,7 @@
 #undef _HAVE_X
 #undef X__Y
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus
 
 // allowlisted definitions
Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -6,6 +6,7 @@
 #define __cplusplus
 #define _HAVE_X 0
 #define X__Y
+#define __STDC__ 1 // expected-warning {{redefining builtin macro}}
 
 #undef for
 #undef final
@@ -13,6 +14,7 @@
 #undef __cplusplus
 #undef _HAVE_X
 #undef X__Y
+#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
 
 // allowlisted definitions
 #define while while
Index: clang/test/Lexer/builtin_redef.c
===
--- clang/test/Lexer/builtin_redef.c
+++ clang/test/Lexer/builtin_redef.c
@@ -1,12 +1,24 @@
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
-// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
+// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
 
 // CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __TIME__ 1234
 // CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __DATE__
+// CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __STDC__ 1
+// CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __STDC_HOSTED__
 
 // CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __TIME__ 1234
+// CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __DATE__
+// CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __STDC__ 1
 // CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __STDC_HOSTED__
 
 int n = __TIME__;
 __DATE__
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -109,52 +109,52 @@
   PED_Elifndef
 };
 
+static bool isFeatureTestMacro(StringRef MacroName) {
+  // list from:
+  // * https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+  // * https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
+  // * man 7 feature_test_macros
+  // The list must be sorted for correct binary search.
+  static constexpr StringRef ReservedMacro[] = {
+  "_ATFILE_SOURCE",
+  "_BSD_SOURCE",
+  "_CRT_NONSTDC_NO_WARNINGS",
+  "_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
+  "_CRT_SECURE_NO_WARNINGS",
+  "_FILE_OFFSET_BITS",
+  "_FORTIFY_SOURCE",
+  "_GLIBCXX_ASSERTIONS",
+  "_GLIBCXX_CONCEPT_CHECKS",
+  "_GLIBCXX_DEBUG",
+  "_GLIBCXX_DEBUG_PEDANTIC",
+  "_GLIBCXX_PARALLEL",
+  "_GLIBCXX_PARALLEL_ASSERTIONS",
+  "_GLIBCXX_SANITIZE_VECTOR",
+  "_GLIBCXX_USE_CXX11_ABI",
+  "_GLIBCXX_USE_DEPRECATED",
+  "_GNU_SOURCE",
+  "_ISOC11_SOURCE",
+  "_ISOC95_SOURCE",
+  "_ISOC99_SOURCE",
+  "_LARGEFILE64_SOURCE",
+  "_POSIX_C_SOURCE",
+  "_REENTRANT",
+  "_SVID_SOURCE",
+  "_THREAD_SAFE",
+  "_XOPEN_SOURCE",
+  "_XOPEN_SOURCE_EXTENDED",
+  "__STDCPP_WANT_MATH_SPEC_FUNCS__",
+  "__STDC_FORMAT_MACROS",
+  };
+  return std::binary_search(std::begin(ReservedMacro), std::end(ReservedMacro),
+MacroName);
+}
+
 static MacroDiag shouldWarnOnMacroDef(Preprocessor , IdentifierInfo *II) {
   const LangOptions  = PP.getLangOpts();
-  if (isReservedInAllContexts(II->isReserved(Lang))) {
-// list from:
-// - https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-15 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: clang/test/Preprocessor/macro-reserved.cpp:15
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus

Why do we diagnose the undef but not the define?


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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-15 Thread John Brawn via Phabricator via cfe-commits 
john.brawn updated this revision to Diff 522215.
john.brawn edited the summary of this revision.
john.brawn added a comment.

I've gone with handling the _GNU_SOURCE problem in the clang-tidy tests by 
re-using the list of feature test macros in shouldWarnOnMacroDef, and not 
warning if there's a match.


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

https://reviews.llvm.org/D144654

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/builtin_redef.c
  clang/test/Preprocessor/macro-reserved.c
  clang/test/Preprocessor/macro-reserved.cpp

Index: clang/test/Preprocessor/macro-reserved.cpp
===
--- clang/test/Preprocessor/macro-reserved.cpp
+++ clang/test/Preprocessor/macro-reserved.cpp
@@ -12,7 +12,7 @@
 #undef _HAVE_X
 #undef X__Y
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus
 
 // allowlisted definitions
Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -6,6 +6,7 @@
 #define __cplusplus
 #define _HAVE_X 0
 #define X__Y
+#define __STDC__ 1 // expected-warning {{redefining builtin macro}}
 
 #undef for
 #undef final
@@ -13,6 +14,7 @@
 #undef __cplusplus
 #undef _HAVE_X
 #undef X__Y
+#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
 
 // allowlisted definitions
 #define while while
Index: clang/test/Lexer/builtin_redef.c
===
--- clang/test/Lexer/builtin_redef.c
+++ clang/test/Lexer/builtin_redef.c
@@ -1,12 +1,24 @@
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
-// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
+// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
 
 // CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __TIME__ 1234
 // CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __DATE__
+// CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __STDC__ 1
+// CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __STDC_HOSTED__
 
 // CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __TIME__ 1234
+// CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __DATE__
+// CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __STDC__ 1
 // CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __STDC_HOSTED__
 
 int n = __TIME__;
 __DATE__
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -109,52 +109,52 @@
   PED_Elifndef
 };
 
+static bool isFeatureTestMacro(StringRef MacroName) {
+  // list from:
+  // - https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+  // - https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
+  // - man 7 feature_test_macros
+  // The list must be sorted for correct binary search.
+  static constexpr StringRef ReservedMacro[] = {
+  "_ATFILE_SOURCE",
+  "_BSD_SOURCE",
+  "_CRT_NONSTDC_NO_WARNINGS",
+  "_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
+  "_CRT_SECURE_NO_WARNINGS",
+  "_FILE_OFFSET_BITS",
+  "_FORTIFY_SOURCE",
+  "_GLIBCXX_ASSERTIONS",
+  "_GLIBCXX_CONCEPT_CHECKS",
+  "_GLIBCXX_DEBUG",
+  "_GLIBCXX_DEBUG_PEDANTIC",
+  "_GLIBCXX_PARALLEL",
+  "_GLIBCXX_PARALLEL_ASSERTIONS",
+  "_GLIBCXX_SANITIZE_VECTOR",
+  "_GLIBCXX_USE_CXX11_ABI",
+  "_GLIBCXX_USE_DEPRECATED",
+  "_GNU_SOURCE",
+  "_ISOC11_SOURCE",
+  "_ISOC95_SOURCE",
+  "_ISOC99_SOURCE",
+  "_LARGEFILE64_SOURCE",
+  "_POSIX_C_SOURCE",
+  "_REENTRANT",
+  "_SVID_SOURCE",
+  "_THREAD_SAFE",
+  "_XOPEN_SOURCE",
+  "_XOPEN_SOURCE_EXTENDED",
+  "__STDCPP_WANT_MATH_SPEC_FUNCS__",
+  "__STDC_FORMAT_MACROS",
+  };
+  return std::binary_search(std::begin(ReservedMacro), std::end(ReservedMacro),
+MacroName);
+}
+
 static MacroDiag shouldWarnOnMacroDef(Preprocessor , IdentifierInfo *II) {
   const

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-12 Thread John Brawn via Phabricator via cfe-commits 
john.brawn planned changes to this revision.
john.brawn added a comment.

The clang-tidy failures in pre-merge are because

- clang-tidy tests use the compile database from building llvm (though you have 
to manually copy it otherwise the tests silently pass without actually running 
the test, which is why I wasn't seeing this locally)
- this has -D_GNU_SOURCE because llvm/cmake/config-ix.cmake adds it to the 
defines
- clang predefines _GNU_SOURCE when compiling C++ (for certain targets)
- therefore we get the warning for a redefined builtin

I'm not sure yet what the best fix is here. gcc has the same behaviour of 
defining _GNU_SOURCE for C++, but does it by adding -D_GNU_SOURCE to the cc1 
command line so it doesn't give a warning on redefine, but it doesn't look like 
copying this behaviour in clang would be easy. I'll think about this some more.


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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-05-11 Thread John Brawn via Phabricator via cfe-commits 
john.brawn updated this revision to Diff 521263.
john.brawn added a comment.

Rebasing now that the patches this depends on have been committed.


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

https://reviews.llvm.org/D144654

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/builtin_redef.c
  clang/test/Preprocessor/macro-reserved.c
  clang/test/Preprocessor/macro-reserved.cpp

Index: clang/test/Preprocessor/macro-reserved.cpp
===
--- clang/test/Preprocessor/macro-reserved.cpp
+++ clang/test/Preprocessor/macro-reserved.cpp
@@ -12,7 +12,7 @@
 #undef _HAVE_X
 #undef X__Y
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus
 
 // allowlisted definitions
Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -6,6 +6,7 @@
 #define __cplusplus
 #define _HAVE_X 0
 #define X__Y
+#define __STDC__ 1 // expected-warning {{redefining builtin macro}}
 
 #undef for
 #undef final
@@ -13,6 +14,7 @@
 #undef __cplusplus
 #undef _HAVE_X
 #undef X__Y
+#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
 
 // allowlisted definitions
 #define while while
Index: clang/test/Lexer/builtin_redef.c
===
--- clang/test/Lexer/builtin_redef.c
+++ clang/test/Lexer/builtin_redef.c
@@ -1,12 +1,24 @@
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
-// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
+// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
 
 // CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __TIME__ 1234
 // CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __DATE__
+// CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __STDC__ 1
+// CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __STDC_HOSTED__
 
 // CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __TIME__ 1234
+// CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __DATE__
+// CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __STDC__ 1
 // CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __STDC_HOSTED__
 
 int n = __TIME__;
 __DATE__
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -319,15 +319,6 @@
 return Diag(MacroNameTok, diag::err_defined_macro_name);
   }
 
-  if (isDefineUndef == MU_Undef) {
-auto *MI = getMacroInfo(II);
-if (MI && MI->isBuiltinMacro()) {
-  // Warn if undefining "__LINE__" and other builtins, per C99 6.10.8/4
-  // and C++ [cpp.predefined]p4], but allow it as an extension.
-  Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);
-}
-  }
-
   // If defining/undefining reserved identifier or a keyword, we need to issue
   // a warning.
   SourceLocation MacroNameLoc = MacroNameTok.getLocation();
@@ -3012,6 +3003,12 @@
   MI->setTokens(Tokens, BP);
   return MI;
 }
+
+static bool isObjCProtectedMacro(const IdentifierInfo *II) {
+  return II->isStr("__strong") || II->isStr("__weak") ||
+ II->isStr("__unsafe_unretained") || II->isStr("__autoreleasing");
+}
+
 /// HandleDefineDirective - Implements \#define.  This consumes the entire macro
 /// line then lets the caller lex the next real token.
 void Preprocessor::HandleDefineDirective(
@@ -3083,15 +3080,9 @@
 // In Objective-C, ignore attempts to directly redefine the builtin
 // definitions of the ownership qualifiers.  It's still possible to
 // #undef them.
-auto isObjCProtectedMacro = [](const IdentifierInfo *II) -> bool {
-  return II->isStr("__strong") ||
- II->isStr("__weak") ||
- II->isStr("__unsafe_unretained") ||
- II->isStr("__autoreleasing");
-};
-   if (getLangOpts().ObjC &&
-SourceMgr.getFileID(OtherMI->getDefinitionLoc())
-  == getPredefinesFileID() &&
+if (getLangOpts().ObjC &&
+

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-03-13 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

It looks like there's still relevant precommit CI failures: 
`clang-tidy/checkers/readability/identifier-naming-case-violation.cpp` looked 
related to this patch.




Comment at: clang/lib/Lex/PPDirectives.cpp:3189-3192
+if ((MI->isBuiltinMacro() ||
+ SourceMgr.isWrittenInBuiltinFile(MI->getDefinitionLoc())) &&
+!(getLangOpts().ObjC && isObjCProtectedMacro(II)))
+  Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);

john.brawn wrote:
> aaron.ballman wrote:
> > Should this diagnostic be suppressed in a system header on the assumption 
> > that system headers are part of the implementation and thus free to 
> > undefine/redefine macros at will?
> It's already suppressed. ShowInSystemHeader in the Diagnostic tablegen class 
> determines if a diagnostic is shown when it happens in a system header, and 
> it's false by default for warnings.
Ah, good point!


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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-03-13 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:3189-3192
+if ((MI->isBuiltinMacro() ||
+ SourceMgr.isWrittenInBuiltinFile(MI->getDefinitionLoc())) &&
+!(getLangOpts().ObjC && isObjCProtectedMacro(II)))
+  Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);

aaron.ballman wrote:
> Should this diagnostic be suppressed in a system header on the assumption 
> that system headers are part of the implementation and thus free to 
> undefine/redefine macros at will?
It's already suppressed. ShowInSystemHeader in the Diagnostic tablegen class 
determines if a diagnostic is shown when it happens in a system header, and 
it's false by default for warnings.


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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-03-09 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:3189-3192
+if ((MI->isBuiltinMacro() ||
+ SourceMgr.isWrittenInBuiltinFile(MI->getDefinitionLoc())) &&
+!(getLangOpts().ObjC && isObjCProtectedMacro(II)))
+  Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);

Should this diagnostic be suppressed in a system header on the assumption that 
system headers are part of the implementation and thus free to 
undefine/redefine macros at will?


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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-03-09 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added a comment.

D145691  should fix the libc++ CI failures.


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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-03-08 Thread John Brawn via Phabricator via cfe-commits 
john.brawn updated this revision to Diff 503305.
john.brawn added a comment.

Add command line test and release note, and run clang-format.


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

https://reviews.llvm.org/D144654

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/builtin_redef.c
  clang/test/Preprocessor/macro-reserved.c
  clang/test/Preprocessor/macro-reserved.cpp

Index: clang/test/Preprocessor/macro-reserved.cpp
===
--- clang/test/Preprocessor/macro-reserved.cpp
+++ clang/test/Preprocessor/macro-reserved.cpp
@@ -12,7 +12,7 @@
 #undef _HAVE_X
 #undef X__Y
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus
 
 // allowlisted definitions
Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -6,6 +6,7 @@
 #define __cplusplus
 #define _HAVE_X 0
 #define X__Y
+#define __STDC__ 1 // expected-warning {{redefining builtin macro}}
 
 #undef for
 #undef final
@@ -13,6 +14,7 @@
 #undef __cplusplus
 #undef _HAVE_X
 #undef X__Y
+#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
 
 // allowlisted definitions
 #define while while
Index: clang/test/Lexer/builtin_redef.c
===
--- clang/test/Lexer/builtin_redef.c
+++ clang/test/Lexer/builtin_redef.c
@@ -1,12 +1,24 @@
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
-// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
-// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-OUT
+// RUN: %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 | FileCheck %s --check-prefix=CHECK-WARN
+// RUN: not %clang_cc1 %s -D__TIME__=1234 -U__DATE__ -D__STDC__=1 -U__STDC_HOSTED__ -E 2>&1 -pedantic-errors | FileCheck %s --check-prefix=CHECK-ERR
 
 // CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __TIME__ 1234
 // CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __DATE__
+// CHECK-WARN: :{{.*}} warning: redefining builtin macro
+// CHECK-WARN-NEXT: #define __STDC__ 1
+// CHECK-WARN: :{{.*}} warning: undefining builtin macro
+// CHECK-WARN-NEXT: #undef __STDC_HOSTED__
 
 // CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __TIME__ 1234
+// CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __DATE__
+// CHECK-ERR: :{{.*}} error: redefining builtin macro
+// CHECK-ERR-NEXT: #define __STDC__ 1
 // CHECK-ERR: :{{.*}} error: undefining builtin macro
+// CHECK-ERR-NEXT: #undef __STDC_HOSTED__
 
 int n = __TIME__;
 __DATE__
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -319,15 +319,6 @@
 return Diag(MacroNameTok, diag::err_defined_macro_name);
   }
 
-  if (isDefineUndef == MU_Undef) {
-auto *MI = getMacroInfo(II);
-if (MI && MI->isBuiltinMacro()) {
-  // Warn if undefining "__LINE__" and other builtins, per C99 6.10.8/4
-  // and C++ [cpp.predefined]p4], but allow it as an extension.
-  Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);
-}
-  }
-
   // If defining/undefining reserved identifier or a keyword, we need to issue
   // a warning.
   SourceLocation MacroNameLoc = MacroNameTok.getLocation();
@@ -3008,6 +2999,12 @@
   MI->setTokens(Tokens, BP);
   return MI;
 }
+
+static bool isObjCProtectedMacro(const IdentifierInfo *II) {
+  return II->isStr("__strong") || II->isStr("__weak") ||
+ II->isStr("__unsafe_unretained") || II->isStr("__autoreleasing");
+}
+
 /// HandleDefineDirective - Implements \#define.  This consumes the entire macro
 /// line then lets the caller lex the next real token.
 void Preprocessor::HandleDefineDirective(
@@ -3079,15 +3076,9 @@
 // In Objective-C, ignore attempts to directly redefine the builtin
 // definitions of the ownership qualifiers.  It's still possible to
 // #undef them.
-auto isObjCProtectedMacro = [](const IdentifierInfo *II) -> bool {
-  return II->isStr("__strong") ||
- II->isStr("__weak") ||
- II->isStr("__unsafe_unretained") ||
- II->isStr("__autoreleasing");
-};
-   if (getLangOpts().ObjC &&
-SourceMgr.getFileID(OtherMI->getDefinitionLoc())
-  == getPredefinesFileID() &&
+if (getLangOpts().ObjC &&
+

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-03-06 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added a comment.

In D144654#4156430 , @aaron.ballman 
wrote:

> This seems to cause precommit CI failures that should be addressed, and it 
> should also have a release note about the fix.

Failures in CI should be fixed by D145397  
and D144651 . I'll also add a release note.

> Also, should this cover things like `clang -U__cplusplus -D__STDC__=1 
> foo.cpp` as well?

Yes, and I'll add tests for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-02-27 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

This seems to cause precommit CI failures that should be addressed, and it 
should also have a release note about the fix.

Also, should this cover things like `clang -U__cplusplus -D__STDC__=1 foo.cpp` 
as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

2023-02-23 Thread John Brawn via Phabricator via cfe-commits 
john.brawn created this revision.
john.brawn added reviewers: rsmith, jansvoboda11, rjmccall.
Herald added a project: All.
john.brawn requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently we warn when MI->isBuiltinMacro, but this is only true for builtin 
macros that require processing when expanding. Checking 
SourceMgr.isWrittenInBuiltinFile in addition to this will mean that we catch 
all builtin macros.

As part of doing this I've also moved the handling of undefining from 
CheckMacroName to HandleUndefDirective, as it doesn't really make sense to 
handle undefining in CheckMacroName but defining in HandleDefineDirective. It 
would be nice to instead handle both in CheckMacroName, but that isn't possible 
as the handling of defines requires looking at what the name is being defined 
to.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144654

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/macro-reserved.c
  clang/test/Preprocessor/macro-reserved.cpp

Index: clang/test/Preprocessor/macro-reserved.cpp
===
--- clang/test/Preprocessor/macro-reserved.cpp
+++ clang/test/Preprocessor/macro-reserved.cpp
@@ -12,7 +12,7 @@
 #undef _HAVE_X
 #undef X__Y
 
-#undef __cplusplus
+#undef __cplusplus // expected-warning {{undefining builtin macro}}
 #define __cplusplus
 
 // allowlisted definitions
Index: clang/test/Preprocessor/macro-reserved.c
===
--- clang/test/Preprocessor/macro-reserved.c
+++ clang/test/Preprocessor/macro-reserved.c
@@ -6,6 +6,7 @@
 #define __cplusplus
 #define _HAVE_X 0
 #define X__Y
+#define __STDC__ 1 // expected-warning {{redefining builtin macro}}
 
 #undef for
 #undef final
@@ -13,6 +14,7 @@
 #undef __cplusplus
 #undef _HAVE_X
 #undef X__Y
+#undef __STDC_HOSTED__ // expected-warning {{undefining builtin macro}}
 
 // allowlisted definitions
 #define while while
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -319,15 +319,6 @@
 return Diag(MacroNameTok, diag::err_defined_macro_name);
   }
 
-  if (isDefineUndef == MU_Undef) {
-auto *MI = getMacroInfo(II);
-if (MI && MI->isBuiltinMacro()) {
-  // Warn if undefining "__LINE__" and other builtins, per C99 6.10.8/4
-  // and C++ [cpp.predefined]p4], but allow it as an extension.
-  Diag(MacroNameTok, diag::ext_pp_undef_builtin_macro);
-}
-  }
-
   // If defining/undefining reserved identifier or a keyword, we need to issue
   // a warning.
   SourceLocation MacroNameLoc = MacroNameTok.getLocation();
@@ -3004,6 +2995,14 @@
   MI->setTokens(Tokens, BP);
   return MI;
 }
+
+static bool isObjCProtectedMacro(const IdentifierInfo *II) {
+  return II->isStr("__strong") ||
+ II->isStr("__weak") ||
+ II->isStr("__unsafe_unretained") ||
+ II->isStr("__autoreleasing");
+}
+
 /// HandleDefineDirective - Implements \#define.  This consumes the entire macro
 /// line then lets the caller lex the next real token.
 void Preprocessor::HandleDefineDirective(
@@ -3075,13 +3074,7 @@
 // In Objective-C, ignore attempts to directly redefine the builtin
 // definitions of the ownership qualifiers.  It's still possible to
 // #undef them.
-auto isObjCProtectedMacro = [](const IdentifierInfo *II) -> bool {
-  return II->isStr("__strong") ||
- II->isStr("__weak") ||
- II->isStr("__unsafe_unretained") ||
- II->isStr("__autoreleasing");
-};
-   if (getLangOpts().ObjC &&
+if (getLangOpts().ObjC &&
 SourceMgr.getFileID(OtherMI->getDefinitionLoc())
   == getPredefinesFileID() &&
 isObjCProtectedMacro(MacroNameTok.getIdentifierInfo())) {
@@ -3107,7 +3100,8 @@
 
   // Warn if defining "__LINE__" and other builtins, per C99 6.10.8/4 and
   // C++ [cpp.predefined]p4, but allow it as an extension.
-  if (OtherMI->isBuiltinMacro())
+  if (OtherMI->isBuiltinMacro() ||
+  SourceMgr.isWrittenInBuiltinFile(OtherMI->getDefinitionLoc()))
 Diag(MacroNameTok, diag::ext_pp_redef_builtin_macro);
   // Macros must be identical.  This means all tokens and whitespace
   // separation must be the same.  C99 6.10.3p2.
@@ -3187,6 +3181,14 @@
 if (!MI->isUsed() && MI->isWarnIfUnused())
   Diag(MI->getDefinitionLoc(), diag::pp_macro_not_used);
 
+// Warn if undefining "__LINE__" and other builtins, per C99 6.10.8/4 and
+// C++ [cpp.predefined]p4, but allow it as an extension. Don't warn if this
+// is an Objective-C builtin macro though.
+if ((MI->isBuiltinMacro() ||
+ SourceMgr.isWrittenInBuiltinFile(MI->getDefinitionLoc())) &&
+!(getLangOpts().ObjC && isObjCProtectedMacro(II)))
+  Diag(MacroNameTok,