public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
@ 2022-12-01  4:31 Longjun Luo
  2022-12-01 17:01 ` Joseph Myers
  0 siblings, 1 reply; 11+ messages in thread
From: Longjun Luo @ 2022-12-01  4:31 UTC (permalink / raw)
  To: gcc-patches; +Cc: luolongjuna, sangyan

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 <luolongjuna@gmail.com>
---
 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2022-12-01  4:31 [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__ Longjun Luo
@ 2022-12-01 17:01 ` Joseph Myers
  2022-12-01 18:23   ` Longjun Luo
  0 siblings, 1 reply; 11+ messages in thread
From: Joseph Myers @ 2022-12-01 17:01 UTC (permalink / raw)
  To: Longjun Luo; +Cc: gcc-patches, sangyan

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

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2022-12-01 17:01 ` Joseph Myers
@ 2022-12-01 18:23   ` Longjun Luo
  2022-12-01 19:07     ` Joseph Myers
  0 siblings, 1 reply; 11+ messages in thread
From: Longjun Luo @ 2022-12-01 18:23 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, sangyan

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]


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:

<command-line>: 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.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2022-12-01 18:23   ` Longjun Luo
@ 2022-12-01 19:07     ` Joseph Myers
  2022-12-01 19:51       ` Longjun Luo
  0 siblings, 1 reply; 11+ messages in thread
From: Joseph Myers @ 2022-12-01 19:07 UTC (permalink / raw)
  To: Longjun Luo; +Cc: gcc-patches, sangyan

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.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2022-12-01 19:07     ` Joseph Myers
@ 2022-12-01 19:51       ` Longjun Luo
  2022-12-01 21:10         ` Joseph Myers
  0 siblings, 1 reply; 11+ messages in thread
From: Longjun Luo @ 2022-12-01 19:51 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, sangyan


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?




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2022-12-01 19:51       ` Longjun Luo
@ 2022-12-01 21:10         ` Joseph Myers
  2023-01-12 16:02           ` Longjun Luo
  2023-01-12 16:05           ` Longjun Luo
  0 siblings, 2 replies; 11+ messages in thread
From: Joseph Myers @ 2022-12-01 21:10 UTC (permalink / raw)
  To: Longjun Luo; +Cc: gcc-patches, sangyan

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.

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

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2022-12-01 21:10         ` Joseph Myers
@ 2023-01-12 16:02           ` Longjun Luo
  2023-04-30 18:30             ` Jeff Law
  2023-01-12 16:05           ` Longjun Luo
  1 sibling, 1 reply; 11+ messages in thread
From: Longjun Luo @ 2023-01-12 16:02 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, sangyan

 From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001
From: Longjun Luo <luolongjuna@gmail.com>
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 <luolongjuna@gmail.com>
---
  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 00000000000..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 __LINE__ "2"         /* { dg-warning "-:\"__LINE__\" redefined" 
} */
+/* { dg-message "-:previous definition" "" { target *-*-* } line_prev } */
+
+#undef __LINE__              /* Undefine while defined.  */
+
+
  /* Check other built-ins with warnings that may be suppressed.  */

  #if !defined(__FILE__) || !defined(__BASE_FILE__)
@@ -66,12 +83,11 @@

  /* Check selected built-ins not affected by warning suppression. */

-#if !defined(__LINE__) || !defined(__INCLUDE_LEVEL__) || 
!defined(__COUNTER__)
+#if !defined(__INCLUDE_LEVEL__) || !defined(__COUNTER__)
  #error "Expected built-in is not defined (built-in macro expectation 
error)"
  /* { 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2022-12-01 21:10         ` Joseph Myers
  2023-01-12 16:02           ` Longjun Luo
@ 2023-01-12 16:05           ` Longjun Luo
  1 sibling, 0 replies; 11+ messages in thread
From: Longjun Luo @ 2023-01-12 16:05 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, sangyan

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2023-01-12 16:02           ` Longjun Luo
@ 2023-04-30 18:30             ` Jeff Law
  2023-04-30 23:14               ` Jeff Law
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2023-04-30 18:30 UTC (permalink / raw)
  To: Longjun Luo, Joseph Myers; +Cc: gcc-patches, sangyan



On 1/12/23 09:02, Longjun Luo via Gcc-patches wrote:
>  From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001
> From: Longjun Luo <luolongjuna@gmail.com>
> 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 <luolongjuna@gmail.com>
> ---
>   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
Thanks.  I added a suitable ChangeLog and pushed this to the trunk.

jeff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2023-04-30 18:30             ` Jeff Law
@ 2023-04-30 23:14               ` Jeff Law
  2023-04-30 23:19                 ` Andrew Pinski
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Law @ 2023-04-30 23:14 UTC (permalink / raw)
  To: Longjun Luo, Joseph Myers; +Cc: gcc-patches, sangyan



On 4/30/23 12:30, Jeff Law wrote:
> 
> 
> On 1/12/23 09:02, Longjun Luo via Gcc-patches wrote:
>>  From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001
>> From: Longjun Luo <luolongjuna@gmail.com>
>> 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 <luolongjuna@gmail.com>
>> ---
>>   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
> Thanks.  I added a suitable ChangeLog and pushed this to the trunk.
This is causing regressions on various targets for a few tests:

lm32-sim: c-c++-common/cpp/pr92296-2.c  -Wc++-compat   (test for 
warnings, line 41)
lm32-sim: gcc.dg/cpp/undef2.c  (test for warnings, line 9)
lm32-sim: gcc.dg/cpp/undef2.c (test for excess errors)

I have reverted this patch from the trunk.  Please address the 
regressions and resubmit.

Thanks,
Jeff


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__
  2023-04-30 23:14               ` Jeff Law
@ 2023-04-30 23:19                 ` Andrew Pinski
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Pinski @ 2023-04-30 23:19 UTC (permalink / raw)
  To: Jeff Law; +Cc: Longjun Luo, Joseph Myers, gcc-patches, sangyan

On Sun, Apr 30, 2023 at 4:15 PM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 4/30/23 12:30, Jeff Law wrote:
> >
> >
> > On 1/12/23 09:02, Longjun Luo via Gcc-patches wrote:
> >>  From 0821df518b264e754d698d399f98be1a62945e32 Mon Sep 17 00:00:00 2001
> >> From: Longjun Luo <luolongjuna@gmail.com>
> >> 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 <luolongjuna@gmail.com>
> >> ---
> >>   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
> > Thanks.  I added a suitable ChangeLog and pushed this to the trunk.
> This is causing regressions on various targets for a few tests:
>
> lm32-sim: c-c++-common/cpp/pr92296-2.c  -Wc++-compat   (test for
> warnings, line 41)
> lm32-sim: gcc.dg/cpp/undef2.c  (test for warnings, line 9)
> lm32-sim: gcc.dg/cpp/undef2.c (test for excess errors)
>
> I have reverted this patch from the trunk.  Please address the
> regressions and resubmit.

From c-c++-common/cpp/pr92296-2.c (line 40):
#pragma push_macro("__LINE__")
#undef __LINE__         /* { dg-warning "undefining" } */

gcc.dg/cpp/undef2.c (line 9):
#undef __LINE__         /* { dg-warning "undefining \"__LINE__\"" } */


These testcases are specifically testing #undef of __LINE__ ...

Should we be still warning for this case or not?

Thanks,
Andrew Pinski


>
> Thanks,
> Jeff
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-04-30 23:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01  4:31 [PATCH] libcpp: suppress builtin macro redefined warnings for __LINE__ Longjun Luo
2022-12-01 17:01 ` Joseph Myers
2022-12-01 18:23   ` Longjun Luo
2022-12-01 19:07     ` Joseph Myers
2022-12-01 19:51       ` Longjun Luo
2022-12-01 21:10         ` Joseph Myers
2023-01-12 16:02           ` Longjun Luo
2023-04-30 18:30             ` Jeff Law
2023-04-30 23:14               ` Jeff Law
2023-04-30 23:19                 ` Andrew Pinski
2023-01-12 16:05           ` Longjun Luo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).