* C/C++ PATCH to add -Wdangling-else option
@ 2016-04-13 14:14 Marek Polacek
2016-04-13 15:11 ` Jakub Jelinek
2016-04-13 15:13 ` Bernd Schmidt
0 siblings, 2 replies; 10+ messages in thread
From: Marek Polacek @ 2016-04-13 14:14 UTC (permalink / raw)
To: GCC Patches, Joseph Myers
This patch is meant to be applied on top of the "Wparentheses overhaul" patch.
I really think that warning about the dangling else problem isn't appropriate
as a part of the -Wparentheses warning, which I think should only deal with
stuff like precedence of operators, i.e. things where ()'s are missing and not
{}'s.
This new warning is, however, a subset of -Wparentheses.
Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
for the next stage1?
2016-04-13 Marek Polacek <polacek@redhat.com>
* c.opt (Wdangling-else): New option.
* c-parser.c (c_parser_if_statement): Replace OPT_Wparentheses with
OPT_Wdangling_else.
* parser.c (cp_parser_selection_statement): Replace OPT_Wparentheses
with OPT_Wdangling_else.
* doc/invoke.texi: Document -Wdangling-else.
* c-c++-common/Wdangling-else-1.c: New test.
* c-c++-common/Wdangling-else-2.c: New test.
* c-c++-common/Wdangling-else-3.c: New test.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 4f86876..5e7bcb8 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -370,6 +370,10 @@ Wctor-dtor-privacy
C++ ObjC++ Var(warn_ctor_dtor_privacy) Warning
Warn when all constructors and destructors are private.
+Wdangling-else
+C ObjC C++ ObjC++ Var(warn_dangling_else) Warning LangEnabledBy(C ObjC C++ ObjC++,Wparentheses)
+Warn about dangling else.
+
Wdate-time
C ObjC C++ ObjC++ CPP(warn_date_time) CppReason(CPP_W_DATE_TIME) Var(cpp_warn_date_time) Init(0) Warning
Warn about __TIME__, __DATE__ and __TIMESTAMP__ usage.
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index cafcb99..1f95446 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -5486,7 +5486,7 @@ c_parser_if_statement (c_parser *parser, bool *if_p, vec<tree> *chain)
/* Diagnose an ambiguous else if if-then-else is nested inside
if-then. */
if (nested_if)
- warning_at (loc, OPT_Wparentheses,
+ warning_at (loc, OPT_Wdangling_else,
"suggest explicit braces to avoid ambiguous %<else%>");
if (warn_duplicated_cond)
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 00e211e..968706f 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -10951,7 +10951,7 @@ cp_parser_selection_statement (cp_parser* parser, bool *if_p,
statement which does have an else clause. We warn
about the potential ambiguity. */
if (nested_if)
- warning_at (EXPR_LOCATION (statement), OPT_Wparentheses,
+ warning_at (EXPR_LOCATION (statement), OPT_Wdangling_else,
"suggest explicit braces to avoid ambiguous"
" %<else%>");
if (warn_duplicated_cond)
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index e9763d4..6253d78 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -257,7 +257,8 @@ Objective-C and Objective-C++ Dialects}.
-Wc90-c99-compat -Wc99-c11-compat @gol
-Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align -Wcast-qual @gol
-Wchar-subscripts -Wclobbered -Wcomment -Wconditionally-supported @gol
--Wconversion -Wcoverage-mismatch -Wno-cpp -Wdate-time -Wdelete-incomplete @gol
+-Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
+-Wdelete-incomplete @gol
-Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol
-Wdisabled-optimization @gol
-Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol
@@ -3974,46 +3975,6 @@ Also warn if a comparison like @code{x<=y<=z} appears; this is
equivalent to @code{(x<=y ? 1 : 0) <= z}, which is a different
interpretation from that of ordinary mathematical notation.
-Also warn about constructions where there may be confusion to which
-@code{if} statement an @code{else} branch belongs. Here is an example of
-such a case:
-
-@smallexample
-@group
-@{
- if (a)
- if (b)
- foo ();
- else
- bar ();
-@}
-@end group
-@end smallexample
-
-In C/C++, every @code{else} branch belongs to the innermost possible
-@code{if} statement, which in this example is @code{if (b)}. This is
-often not what the programmer expected, as illustrated in the above
-example by indentation the programmer chose. When there is the
-potential for this confusion, GCC issues a warning when this flag
-is specified. To eliminate the warning, add explicit braces around
-the innermost @code{if} statement so there is no way the @code{else}
-can belong to the enclosing @code{if}. The resulting code
-looks like this:
-
-@smallexample
-@group
-@{
- if (a)
- @{
- if (b)
- foo ();
- else
- bar ();
- @}
-@}
-@end group
-@end smallexample
-
Also warn for dangerous uses of the GNU extension to
@code{?:} with omitted middle operand. When the condition
in the @code{?}: operator is a boolean expression, the omitted value is
@@ -5146,6 +5107,51 @@ compiler doesn't give this warning for types defined in the main .C
file, as those are unlikely to have multiple definitions.
@option{-Wsubobject-linkage} is enabled by default.
+@item -Wdangling-else
+@opindex Wdangling-else
+@opindex Wno-dangling-else
+Warn about constructions where there may be confusion to which
+@code{if} statement an @code{else} branch belongs. Here is an example of
+such a case:
+
+@smallexample
+@group
+@{
+ if (a)
+ if (b)
+ foo ();
+ else
+ bar ();
+@}
+@end group
+@end smallexample
+
+In C/C++, every @code{else} branch belongs to the innermost possible
+@code{if} statement, which in this example is @code{if (b)}. This is
+often not what the programmer expected, as illustrated in the above
+example by indentation the programmer chose. When there is the
+potential for this confusion, GCC issues a warning when this flag
+is specified. To eliminate the warning, add explicit braces around
+the innermost @code{if} statement so there is no way the @code{else}
+can belong to the enclosing @code{if}. The resulting code
+looks like this:
+
+@smallexample
+@group
+@{
+ if (a)
+ @{
+ if (b)
+ foo ();
+ else
+ bar ();
+ @}
+@}
+@end group
+@end smallexample
+
+This warning is enabled by @option{-Wparentheses}.
+
@item -Wdate-time
@opindex Wdate-time
@opindex Wno-date-time
diff --git gcc/testsuite/c-c++-common/Wdangling-else-1.c gcc/testsuite/c-c++-common/Wdangling-else-1.c
index e69de29..28a5a8f 100644
--- gcc/testsuite/c-c++-common/Wdangling-else-1.c
+++ gcc/testsuite/c-c++-common/Wdangling-else-1.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-Wdangling-else" } */
+
+void bar (int);
+void
+foo (int a, int b)
+{
+ if (a) /* { dg-warning "suggest explicit braces to avoid ambiguous" } */
+ if (b)
+ bar (1);
+ else
+ bar (2);
+}
diff --git gcc/testsuite/c-c++-common/Wdangling-else-2.c gcc/testsuite/c-c++-common/Wdangling-else-2.c
index e69de29..87ea1ab 100644
--- gcc/testsuite/c-c++-common/Wdangling-else-2.c
+++ gcc/testsuite/c-c++-common/Wdangling-else-2.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-Wparentheses" } */
+
+void bar (int);
+void
+foo (int a, int b)
+{
+ if (a) /* { dg-warning "suggest explicit braces to avoid ambiguous" } */
+ if (b)
+ bar (1);
+ else
+ bar (2);
+}
diff --git gcc/testsuite/c-c++-common/Wdangling-else-3.c gcc/testsuite/c-c++-common/Wdangling-else-3.c
index e69de29..0dae0d5 100644
--- gcc/testsuite/c-c++-common/Wdangling-else-3.c
+++ gcc/testsuite/c-c++-common/Wdangling-else-3.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-Wparentheses -Wno-dangling-else" } */
+
+void bar (int);
+void
+foo (int a, int b)
+{
+ if (a) /* { dg-bogus "suggest explicit braces to avoid ambiguous" } */
+ if (b)
+ bar (1);
+ else
+ bar (2);
+}
Marek
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C/C++ PATCH to add -Wdangling-else option
2016-04-13 14:14 C/C++ PATCH to add -Wdangling-else option Marek Polacek
@ 2016-04-13 15:11 ` Jakub Jelinek
2016-04-13 15:13 ` Bernd Schmidt
1 sibling, 0 replies; 10+ messages in thread
From: Jakub Jelinek @ 2016-04-13 15:11 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers
On Wed, Apr 13, 2016 at 04:14:44PM +0200, Marek Polacek wrote:
> This patch is meant to be applied on top of the "Wparentheses overhaul" patch.
>
> I really think that warning about the dangling else problem isn't appropriate
> as a part of the -Wparentheses warning, which I think should only deal with
> stuff like precedence of operators, i.e. things where ()'s are missing and not
> {}'s.
>
> This new warning is, however, a subset of -Wparentheses.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
> for the next stage1?
>
> 2016-04-13 Marek Polacek <polacek@redhat.com>
>
> * c.opt (Wdangling-else): New option.
>
> * c-parser.c (c_parser_if_statement): Replace OPT_Wparentheses with
> OPT_Wdangling_else.
>
> * parser.c (cp_parser_selection_statement): Replace OPT_Wparentheses
> with OPT_Wdangling_else.
>
> * doc/invoke.texi: Document -Wdangling-else.
>
> * c-c++-common/Wdangling-else-1.c: New test.
> * c-c++-common/Wdangling-else-2.c: New test.
> * c-c++-common/Wdangling-else-3.c: New test.
LGTM, though I think it would be useful to include in invoke.texi also a
small example with for to make it clear the option also handles that.
Jakub
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C/C++ PATCH to add -Wdangling-else option
2016-04-13 14:14 C/C++ PATCH to add -Wdangling-else option Marek Polacek
2016-04-13 15:11 ` Jakub Jelinek
@ 2016-04-13 15:13 ` Bernd Schmidt
2016-04-13 15:16 ` Jakub Jelinek
1 sibling, 1 reply; 10+ messages in thread
From: Bernd Schmidt @ 2016-04-13 15:13 UTC (permalink / raw)
To: Marek Polacek, GCC Patches, Joseph Myers
On 04/13/2016 04:14 PM, Marek Polacek wrote:
> This patch is meant to be applied on top of the "Wparentheses overhaul" patch.
>
> I really think that warning about the dangling else problem isn't appropriate
> as a part of the -Wparentheses warning, which I think should only deal with
> stuff like precedence of operators, i.e. things where ()'s are missing and not
> {}'s.
>
> This new warning is, however, a subset of -Wparentheses.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
> for the next stage1?
I think it's not appropriate for now. I'm ambivalent about the concept;
my (vague) recollection is that putting it under -Wparentheses was
Kenner's idea, and it's been there so long that I'm not sure there's
really a point to changing this. In a sense it is a very similar problem
as operator precedence.
Bernd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C/C++ PATCH to add -Wdangling-else option
2016-04-13 15:13 ` Bernd Schmidt
@ 2016-04-13 15:16 ` Jakub Jelinek
2016-04-26 12:32 ` Marek Polacek
0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2016-04-13 15:16 UTC (permalink / raw)
To: Bernd Schmidt; +Cc: Marek Polacek, GCC Patches, Joseph Myers
On Wed, Apr 13, 2016 at 05:13:28PM +0200, Bernd Schmidt wrote:
> On 04/13/2016 04:14 PM, Marek Polacek wrote:
> >This patch is meant to be applied on top of the "Wparentheses overhaul" patch.
> >
> >I really think that warning about the dangling else problem isn't appropriate
> >as a part of the -Wparentheses warning, which I think should only deal with
> >stuff like precedence of operators, i.e. things where ()'s are missing and not
> >{}'s.
> >
> >This new warning is, however, a subset of -Wparentheses.
> >
> >Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
> >for the next stage1?
>
> I think it's not appropriate for now. I'm ambivalent about the concept; my
> (vague) recollection is that putting it under -Wparentheses was Kenner's
> idea, and it's been there so long that I'm not sure there's really a point
> to changing this. In a sense it is a very similar problem as operator
> precedence.
Well, even with the change it is still included with -Wparentheses, just
it is a suboption with more specific name that can be enabled/disabled
independently from -Wparentheses if needed.
Though, of course, it can wait for GCC 7.
Jakub
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C/C++ PATCH to add -Wdangling-else option
2016-04-13 15:16 ` Jakub Jelinek
@ 2016-04-26 12:32 ` Marek Polacek
2016-04-26 12:39 ` Jakub Jelinek
0 siblings, 1 reply; 10+ messages in thread
From: Marek Polacek @ 2016-04-26 12:32 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Bernd Schmidt, GCC Patches, Joseph Myers
On Wed, Apr 13, 2016 at 05:16:12PM +0200, Jakub Jelinek wrote:
> On Wed, Apr 13, 2016 at 05:13:28PM +0200, Bernd Schmidt wrote:
> > On 04/13/2016 04:14 PM, Marek Polacek wrote:
> > >This patch is meant to be applied on top of the "Wparentheses overhaul" patch.
> > >
> > >I really think that warning about the dangling else problem isn't appropriate
> > >as a part of the -Wparentheses warning, which I think should only deal with
> > >stuff like precedence of operators, i.e. things where ()'s are missing and not
> > >{}'s.
> > >
> > >This new warning is, however, a subset of -Wparentheses.
> > >
> > >Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
> > >for the next stage1?
> >
> > I think it's not appropriate for now. I'm ambivalent about the concept; my
> > (vague) recollection is that putting it under -Wparentheses was Kenner's
> > idea, and it's been there so long that I'm not sure there's really a point
> > to changing this. In a sense it is a very similar problem as operator
> > precedence.
>
> Well, even with the change it is still included with -Wparentheses, just
> it is a suboption with more specific name that can be enabled/disabled
> independently from -Wparentheses if needed.
> Though, of course, it can wait for GCC 7.
So how do y'all feel about this patch now that we're in stage1?
Marek
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C/C++ PATCH to add -Wdangling-else option
2016-04-26 12:32 ` Marek Polacek
@ 2016-04-26 12:39 ` Jakub Jelinek
2016-04-26 13:03 ` Bernd Schmidt
0 siblings, 1 reply; 10+ messages in thread
From: Jakub Jelinek @ 2016-04-26 12:39 UTC (permalink / raw)
To: Marek Polacek; +Cc: Bernd Schmidt, GCC Patches, Joseph Myers
On Tue, Apr 26, 2016 at 02:32:01PM +0200, Marek Polacek wrote:
> On Wed, Apr 13, 2016 at 05:16:12PM +0200, Jakub Jelinek wrote:
> > On Wed, Apr 13, 2016 at 05:13:28PM +0200, Bernd Schmidt wrote:
> > > On 04/13/2016 04:14 PM, Marek Polacek wrote:
> > > >This patch is meant to be applied on top of the "Wparentheses overhaul" patch.
> > > >
> > > >I really think that warning about the dangling else problem isn't appropriate
> > > >as a part of the -Wparentheses warning, which I think should only deal with
> > > >stuff like precedence of operators, i.e. things where ()'s are missing and not
> > > >{}'s.
> > > >
> > > >This new warning is, however, a subset of -Wparentheses.
> > > >
> > > >Bootstrapped/regtested on x86_64-linux, ok for trunk or should I stash it
> > > >for the next stage1?
> > >
> > > I think it's not appropriate for now. I'm ambivalent about the concept; my
> > > (vague) recollection is that putting it under -Wparentheses was Kenner's
> > > idea, and it's been there so long that I'm not sure there's really a point
> > > to changing this. In a sense it is a very similar problem as operator
> > > precedence.
> >
> > Well, even with the change it is still included with -Wparentheses, just
> > it is a suboption with more specific name that can be enabled/disabled
> > independently from -Wparentheses if needed.
> > Though, of course, it can wait for GCC 7.
>
> So how do y'all feel about this patch now that we're in stage1?
I support that change, and -Wparentheses will still enable this, it just
gives more fine-grained control and be in line with what clang does.
Bernd, how much are you against this change?
Jakub
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C/C++ PATCH to add -Wdangling-else option
2016-04-26 12:39 ` Jakub Jelinek
@ 2016-04-26 13:03 ` Bernd Schmidt
2016-05-04 13:19 ` Marek Polacek
0 siblings, 1 reply; 10+ messages in thread
From: Bernd Schmidt @ 2016-04-26 13:03 UTC (permalink / raw)
To: Jakub Jelinek, Marek Polacek; +Cc: GCC Patches, Joseph Myers
On 04/26/2016 02:39 PM, Jakub Jelinek wrote:
> I support that change, and -Wparentheses will still enable this, it just
> gives more fine-grained control and be in line with what clang does.
>
> Bernd, how much are you against this change?
Don't really care that much, I just don't quite see the point. Don't let
me stop you though.
Bernd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C/C++ PATCH to add -Wdangling-else option
2016-04-26 13:03 ` Bernd Schmidt
@ 2016-05-04 13:19 ` Marek Polacek
2016-05-04 15:39 ` Joseph Myers
0 siblings, 1 reply; 10+ messages in thread
From: Marek Polacek @ 2016-05-04 13:19 UTC (permalink / raw)
To: Bernd Schmidt; +Cc: Jakub Jelinek, GCC Patches, Joseph Myers
On Tue, Apr 26, 2016 at 03:03:25PM +0200, Bernd Schmidt wrote:
> On 04/26/2016 02:39 PM, Jakub Jelinek wrote:
> > I support that change, and -Wparentheses will still enable this, it just
> > gives more fine-grained control and be in line with what clang does.
> >
> > Bernd, how much are you against this change?
>
> Don't really care that much, I just don't quite see the point. Don't let me
> stop you though.
So Joseph, what do you think about this patch? :)
Marek
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C/C++ PATCH to add -Wdangling-else option
2016-05-04 13:19 ` Marek Polacek
@ 2016-05-04 15:39 ` Joseph Myers
2016-05-04 15:43 ` Marek Polacek
0 siblings, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2016-05-04 15:39 UTC (permalink / raw)
To: Marek Polacek; +Cc: Bernd Schmidt, Jakub Jelinek, GCC Patches
On Wed, 4 May 2016, Marek Polacek wrote:
> On Tue, Apr 26, 2016 at 03:03:25PM +0200, Bernd Schmidt wrote:
> > On 04/26/2016 02:39 PM, Jakub Jelinek wrote:
> > > I support that change, and -Wparentheses will still enable this, it just
> > > gives more fine-grained control and be in line with what clang does.
> > >
> > > Bernd, how much are you against this change?
> >
> > Don't really care that much, I just don't quite see the point. Don't let me
> > stop you though.
>
> So Joseph, what do you think about this patch? :)
I support adding the option for more fine-grained control of these
warnings.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: C/C++ PATCH to add -Wdangling-else option
2016-05-04 15:39 ` Joseph Myers
@ 2016-05-04 15:43 ` Marek Polacek
0 siblings, 0 replies; 10+ messages in thread
From: Marek Polacek @ 2016-05-04 15:43 UTC (permalink / raw)
To: Joseph Myers; +Cc: Bernd Schmidt, Jakub Jelinek, GCC Patches
On Wed, May 04, 2016 at 03:39:19PM +0000, Joseph Myers wrote:
> On Wed, 4 May 2016, Marek Polacek wrote:
>
> > On Tue, Apr 26, 2016 at 03:03:25PM +0200, Bernd Schmidt wrote:
> > > On 04/26/2016 02:39 PM, Jakub Jelinek wrote:
> > > > I support that change, and -Wparentheses will still enable this, it just
> > > > gives more fine-grained control and be in line with what clang does.
> > > >
> > > > Bernd, how much are you against this change?
> > >
> > > Don't really care that much, I just don't quite see the point. Don't let me
> > > stop you though.
> >
> > So Joseph, what do you think about this patch? :)
>
> I support adding the option for more fine-grained control of these
> warnings.
Thanks. I'll commit the patch then.
Marek
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-05-04 15:43 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-13 14:14 C/C++ PATCH to add -Wdangling-else option Marek Polacek
2016-04-13 15:11 ` Jakub Jelinek
2016-04-13 15:13 ` Bernd Schmidt
2016-04-13 15:16 ` Jakub Jelinek
2016-04-26 12:32 ` Marek Polacek
2016-04-26 12:39 ` Jakub Jelinek
2016-04-26 13:03 ` Bernd Schmidt
2016-05-04 13:19 ` Marek Polacek
2016-05-04 15:39 ` Joseph Myers
2016-05-04 15:43 ` Marek Polacek
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).