[v2,4/4] gdb: Suppress "unused" variable warning on Clang

Message ID 189b22f1db46c1ffbb248aeca8b5753da4070f2c.1663835104.git.research_trasio@irq.a4lg.com
State Superseded
Headers
Series gdb: (includes PR28413), Suppress some general warnings if built with Clang |

Commit Message

Tsukasa OI Sept. 22, 2022, 8:25 a.m. UTC
  Clang generates a warning if there is a variable which is written but not
read thereafter.  By the default configuration (with "-Werror"), it causes a
build failure (unless "--disable-werror" is specified).

Because the cause of this error is in the Bison-generated code
($(srcdir)/gdb/cp-name-parser.y -> $(builddir)/gdb/cp-name-parser.c),
this commit suppresses this warning ("-Wunused-but-set-variable") by placing
the DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro at the end of user
prologue of cp-name-parser.y.

gdb/ChangeLog:

	* cp-name-parser.y: Suppress -Wunused-but-set-variable warning on
	the Bison-generated code.
---
 gdb/cp-name-parser.y | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Simon Marchi Oct. 12, 2022, 5:36 p.m. UTC | #1
On 9/22/22 04:25, Tsukasa OI via Gdb-patches wrote:
> Clang generates a warning if there is a variable which is written but not
> read thereafter.  By the default configuration (with "-Werror"), it causes a
> build failure (unless "--disable-werror" is specified).
> 
> Because the cause of this error is in the Bison-generated code
> ($(srcdir)/gdb/cp-name-parser.y -> $(builddir)/gdb/cp-name-parser.c),
> this commit suppresses this warning ("-Wunused-but-set-variable") by placing
> the DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro at the end of user
> prologue of cp-name-parser.y.

Hi,

I just sent a patch to fix the same thing as your patch here, but only
saw your patch after.  However, I took a different, more fine-grained
approach:

https://inbox.sourceware.org/gdb-patches/20221012173256.20079-1-simon.marchi@efficios.com/T/#u

Can you let me know what you think?

Simon
  
Tsukasa OI Oct. 16, 2022, 1:37 p.m. UTC | #2
On 2022/10/13 2:36, Simon Marchi wrote:
> On 9/22/22 04:25, Tsukasa OI via Gdb-patches wrote:
>> Clang generates a warning if there is a variable which is written but not
>> read thereafter.  By the default configuration (with "-Werror"), it causes a
>> build failure (unless "--disable-werror" is specified).
>>
>> Because the cause of this error is in the Bison-generated code
>> ($(srcdir)/gdb/cp-name-parser.y -> $(builddir)/gdb/cp-name-parser.c),
>> this commit suppresses this warning ("-Wunused-but-set-variable") by placing
>> the DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro at the end of user
>> prologue of cp-name-parser.y.
> 
> Hi,
> 
> I just sent a patch to fix the same thing as your patch here, but only
> saw your patch after.  However, I took a different, more fine-grained
> approach:
> 
> https://inbox.sourceware.org/gdb-patches/20221012173256.20079-1-simon.marchi@efficios.com/T/#u
> 
> Can you let me know what you think?
> 
> Simon
> 

I always prefer fine-grained approach if we can.  The only reason I
didn't do that on gold is because I didn't know proper place for "(void)
yynerrs".  I cannot approve (as I'm not a maintainer) but I support your
patchset.

Thanks,
Tsukasa
  
Simon Marchi Oct. 17, 2022, 12:35 p.m. UTC | #3
On 2022-10-16 09:37, Tsukasa OI via Gdb-patches wrote:
> On 2022/10/13 2:36, Simon Marchi wrote:
>> On 9/22/22 04:25, Tsukasa OI via Gdb-patches wrote:
>>> Clang generates a warning if there is a variable which is written but not
>>> read thereafter.  By the default configuration (with "-Werror"), it causes a
>>> build failure (unless "--disable-werror" is specified).
>>>
>>> Because the cause of this error is in the Bison-generated code
>>> ($(srcdir)/gdb/cp-name-parser.y -> $(builddir)/gdb/cp-name-parser.c),
>>> this commit suppresses this warning ("-Wunused-but-set-variable") by placing
>>> the DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro at the end of user
>>> prologue of cp-name-parser.y.
>>
>> Hi,
>>
>> I just sent a patch to fix the same thing as your patch here, but only
>> saw your patch after.  However, I took a different, more fine-grained
>> approach:
>>
>> https://inbox.sourceware.org/gdb-patches/20221012173256.20079-1-simon.marchi@efficios.com/T/#u
>>
>> Can you let me know what you think?
>>
>> Simon
>>
> 
> I always prefer fine-grained approach if we can.  The only reason I
> didn't do that on gold is because I didn't know proper place for "(void)
> yynerrs".  I cannot approve (as I'm not a maintainer) but I support your
> patchset.

Ok, thanks for the feedback, I will push my version.

Simon
  

Patch

diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 34c691ddabb..21ba51679d3 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -248,6 +248,10 @@  cpname_state::make_name (const char *name, int len)
 
 static int yylex (YYSTYPE *, cpname_state *);
 static void yyerror (cpname_state *, const char *);
+
+#include "diagnostics.h"
+DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE
+
 %}
 
 %type <comp> exp exp1 type start start_opt oper colon_name