Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
On 12/2/2022 5:10 AM, Joseph Myers wrote: On Fri, 2 Dec 2022, Longjun Luo via Gcc-patches wrote: They are ./gcc/testsuite/gcc.dg/cpp/warn-redefined.c and ./gcc/testsuite/gcc.dg/cpp/warn-redefined-2.c These two cases redefine the __TIME__ macro when using the option '-Wbuiltin-macro-redefined'. I think I shoud add a test to verify __LINE__ macro in these two cases. I think it should be a test that doesn't use either -Wbuiltin-macro-redefined or -Wno-builtin-macro-redefined - a test of how the compiler behaves by default. Sorry for the delay. I have added a test case for this default situation. And also update another case to fully test the usage of builtin macros redefintions. So, the patch itself has no problem. What I need do is to rich its test cases and update change log, right? The patch needs review, but I'm fine with the principle that -Wno-builtin-macro-redefined should apply to __LINE__ as it does to various other built-in macros. Thanks for your patience.
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001 From: Longjun Luo Date: Thu, 12 Jan 2023 23:59:54 +0800 Subject: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__ As implied in gcc.gnu.org/legacy-ml/gcc-patches/2008-09/msg00076.html, gcc provides -Wno-builtin-macro-redefined to suppress warning when redefining builtin macro. However, at that time, there was no scenario for __LINE__ macro. But, when we try to build a live-patch, we compare sections by using -ffunction-sections. Some same functions are considered changed because of __LINE__ macro. At present, to detect such a changed caused by __LINE__ macro, we have to analyse code and maintain a function list. For example, in kpatch, check this commit github.com/dynup/kpatch/commit/0e1b95edeafa36edb7bcf11da6d1c00f76d7e03d. So, in this scenario, when we try to compared sections, it would be better to support suppress builtin macro redefined warnings for __LINE__ macro. Signed-off-by: Longjun Luo --- gcc/testsuite/gcc.dg/builtin-redefine-1.c | 49 +++ gcc/testsuite/gcc.dg/builtin-redefine.c | 24 +-- libcpp/init.cc| 2 +- 3 files changed, 70 insertions(+), 5 deletions(-) create mode 100755 gcc/testsuite/gcc.dg/builtin-redefine-1.c diff --git a/gcc/testsuite/gcc.dg/builtin-redefine-1.c b/gcc/testsuite/gcc.dg/builtin-redefine-1.c new file mode 100755 index 000..c1e05b4fc7c --- /dev/null +++ b/gcc/testsuite/gcc.dg/builtin-redefine-1.c @@ -0,0 +1,49 @@ +/* Test default warnings for redefining builtin macros. */ + +/* { dg-do compile } */ +/* { dg-options "-D__TIMESTAMP__=x -D__TIME__=x -D__DATE__=x -D__FILE__=x -D__FILE_NAME__=x -D__BASE_FILE__=x -D__LINE__=0" } */ + +/* Check default behavior for builtin macros redefinition. */ + +/* { dg-message "\"__TIMESTAMP__\" redefined" "" {target "*-*-*"} 0 } */ +#ifndef __TIMESTAMP__ +#error "__TIMESTAMP__ builtin is not defined" +/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ +#endif + +/* { dg-message "\"__TIME__\" redefined" "" {target "*-*-*"} 0 } */ +#ifndef __TIME__ +#error "__TIME__ builtin is not defined" +/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ +#endif + +/* { dg-message "\"__DATE__\" redefined" "" {target "*-*-*"} 0 } */ +#ifndef __DATE__ +#error "__DATE__ builtin is not defined" +/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ +#endif + +/* { dg-message "\"__FILE__\" redefined" "" {target "*-*-*"} 0 } */ +#ifndef __FILE__ +#error "__FILE__ builtin is not defined" +/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ +#endif + +/* { dg-message "\"__FILE_NAME__\" redefined" "" {target "*-*-*"} 0 } */ +#ifndef __FILE_NAME__ +#error "__FILE_NAME__ builtin is not defined" +/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ +#endif + +/* { dg-message "\"__BASE_FILE__\" redefined" "" {target "*-*-*"} 0 } */ +#ifndef __BASE_FILE__ +#error "__BASE_FILE__ builtin is not defined" +/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ +#endif + +/* { dg-message "\"__LINE__\" redefined" "" {target "*-*-*"} 0 } */ +#ifndef __LINE__ +#error "__LINE__ builtin is not defined" +/* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ +#endif + diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c b/gcc/testsuite/gcc.dg/builtin-redefine.c index 882b2210992..fa27ee9aefc 100644 --- a/gcc/testsuite/gcc.dg/builtin-redefine.c +++ b/gcc/testsuite/gcc.dg/builtin-redefine.c @@ -1,9 +1,9 @@ /* Test -Wno-builtin-macro-redefined warnings. */ /* { dg-do compile } */ -/* { dg-options "-Wno-builtin-macro-redefined -U__DATE__ -D__TIME__=X" } */ +/* { dg-options "-Wno-builtin-macro-redefined -U__DATE__ -D__TIME__=X -D__LINE__=0" } */ -/* Check date, time, and datestamp built-ins warnings may be suppressed. */ +/* Check date, time, datestamp and line built-ins warnings may be suppressed. */ #if defined(__DATE__) #error "__DATE__ is defined, but should not be (-U command line error)" @@ -15,6 +15,11 @@ /* { dg-bogus "__TIME__ is not defined" "" { target *-*-* } .-1 } */ #endif +#if __LINE__ != 0 +#error "__LINE__ is not defined as expected (-D command line error)" +/* { dg-bogus "__LINE__ is not defined" "" { target *-*-* } .-1 } */ +#endif + #if !defined(__TIMESTAMP__) #error "__TIMESTAMP__ is not defined (built-in macro expectation error)" /* { dg-bogus "__TIMESTAMP__ is not defined" "" { target *-*-* } .-1 } */ @@ -53,6 +58,18 @@ #undef __TIMESTAMP__ /* Undefine while defined. */ +#undef __LINE__ /* Undefine while defined. */ +#undef __LINE__ /* Undefine while already undefined. */ + +#define __LINE__ "1" /* Define while undefined. */ +#define __LINE__ "1" /* Re-define while defined. */ /* { dg-line line_prev } */ + +#define __LI
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
On 12/2/2022 3:07 AM, Joseph Myers wrote: On Fri, 2 Dec 2022, Longjun Luo via Gcc-patches wrote: On 12/2/2022 1:01 AM, Joseph Myers wrote: On Thu, 1 Dec 2022, Longjun Luo via Gcc-patches wrote: diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c b/gcc/testsuite/gcc.dg/builtin-redefine.c index 882b2210992..9d5b42252ee 100644 --- a/gcc/testsuite/gcc.dg/builtin-redefine.c +++ b/gcc/testsuite/gcc.dg/builtin-redefine.c @@ -71,7 +71,6 @@ /* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ #endif -#define __LINE__ 0 /* { dg-warning "-:\"__LINE__\" redef" } */ #define __INCLUDE_LEVEL__ 0 /* { dg-warning "-:\"__INCLUDE_LEVEL__\" redef" } */ #define __COUNTER__ 0/* { dg-warning "-:\"__COUNTER__\" redef" } */ Is there some existing test that verifies that this redefinition is still diagnosed by default (in the absence of -Wno-builtin-macro-redefined)? I am not sure I have fully understood your meaning. The problem here is that if I try to redefine __LINE__ macro in the situation that projects use the option '-Werror', the compile will fail. There are two cases: (a) Is redefinition of __LINE__ diagnosed *without* -Wno-builtin-macro-redefined? (b) Is redefinition of __LINE__ diagnosed *with* -Wno-builtin-macro-redefined? My understanding is that both (a) and (b) have answer "yes" at present, and your patch would change the answer to (b) to "no", without changing the answer to (a). My question is about whether there is a test verifying the answer to (a). If not, I think the patch should add one. After some check for the source code, two similiar exist test cases for the situation (a). They are ./gcc/testsuite/gcc.dg/cpp/warn-redefined.c and ./gcc/testsuite/gcc.dg/cpp/warn-redefined-2.c These two cases redefine the __TIME__ macro when using the option '-Wbuiltin-macro-redefined'. I think I shoud add a test to verify __LINE__ macro in these two cases. I will write a complete test for situation (a) and situation (b). But I need a little time to be familar with the gcc testcases. So, the patch itself has no problem. What I need do is to rich its test cases and update change log, right?
Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
On 12/2/2022 1:01 AM, Joseph Myers wrote: On Thu, 1 Dec 2022, Longjun Luo via Gcc-patches wrote: diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c b/gcc/testsuite/gcc.dg/builtin-redefine.c index 882b2210992..9d5b42252ee 100644 --- a/gcc/testsuite/gcc.dg/builtin-redefine.c +++ b/gcc/testsuite/gcc.dg/builtin-redefine.c @@ -71,7 +71,6 @@ /* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ #endif -#define __LINE__ 0 /* { dg-warning "-:\"__LINE__\" redef" } */ #define __INCLUDE_LEVEL__ 0 /* { dg-warning "-:\"__INCLUDE_LEVEL__\" redef" } */ #define __COUNTER__ 0/* { dg-warning "-:\"__COUNTER__\" redef" } */ Is there some existing test that verifies that this redefinition is still diagnosed by default (in the absence of -Wno-builtin-macro-redefined)? I am not sure I have fully understood your meaning. The problem here is that if I try to redefine __LINE__ macro in the situation that projects use the option '-Werror', the compile will fail. For example, the following compilation will fail: /echo "void main(){}" | gcc -D__LINE__=0 -Werror -x c -/ The compilation output is: : error: "__LINE__" redefined [-Werror] cc1: all warnings being treated as errors As I know, most projects including Linux kernel enable '-Werror' by default. So if I try to redefine __LINE__ macro in this situation, it will be impossible. The reason that I want to redefine __LINE__ macro has been explained in the commit. Thanks for your patience and hope I hit the point.
[PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
As implied in gcc.gnu.org/legacy-ml/gcc-patches/2008-09/msg00076.html, gcc provides -Wno-builtin-macro-redefined to suppress warning when redefining builtin macro. However, at that time, there was no scenario for __LINE__ macro. But, when we try to build a live-patch, we compare sections by using -ffunction-sections. Some same functions are considered changed because of __LINE__ macro. At present, to detect such a changed caused by __LINE__ macro, we have to analyse code and maintain a function list. For example, in kpatch, check this commit github.com/dynup/kpatch/commit/0e1b95edeafa36edb7bcf11da6d1c00f76d7e03d. So, in this scenario, when we try to compared sections, it would be better to support suppress builtin macro redefined warnings for __LINE__ macro. Signed-off-by: Longjun Luo --- gcc/testsuite/gcc.dg/builtin-redefine.c | 1 - libcpp/init.cc | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.dg/builtin-redefine.c b/gcc/testsuite/gcc.dg/builtin-redefine.c index 882b2210992..9d5b42252ee 100644 --- a/gcc/testsuite/gcc.dg/builtin-redefine.c +++ b/gcc/testsuite/gcc.dg/builtin-redefine.c @@ -71,7 +71,6 @@ /* { dg-bogus "Expected built-in is not defined" "" { target *-*-* } .-1 } */ #endif -#define __LINE__ 0 /* { dg-warning "-:\"__LINE__\" redef" } */ #define __INCLUDE_LEVEL__ 0 /* { dg-warning "-:\"__INCLUDE_LEVEL__\" redef" } */ #define __COUNTER__ 0/* { dg-warning "-:\"__COUNTER__\" redef" } */ diff --git a/libcpp/init.cc b/libcpp/init.cc index 5f34e3515d2..2765b9838b7 100644 --- a/libcpp/init.cc +++ b/libcpp/init.cc @@ -421,7 +421,7 @@ static const struct builtin_macro builtin_array[] = B("__FILE__", BT_FILE, false), B("__FILE_NAME__",BT_FILE_NAME, false), B("__BASE_FILE__",BT_BASE_FILE, false), - B("__LINE__", BT_SPECLINE, true), + B("__LINE__", BT_SPECLINE, false), B("__INCLUDE_LEVEL__", BT_INCLUDE_LEVEL, true), B("__COUNTER__", BT_COUNTER, true), /* Make sure to update the list of built-in -- 2.38.1