From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18364 invoked by alias); 13 Apr 2016 14:14:52 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 18352 invoked by uid 89); 13 Apr 2016 14:14:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Wcastqual, wcast-qual, Wcast-qual, wcastqual X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 13 Apr 2016 14:14:49 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0878FC04B315; Wed, 13 Apr 2016 14:14:48 +0000 (UTC) Received: from redhat.com (ovpn-204-18.brq.redhat.com [10.40.204.18]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3DEEi8L012346 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 13 Apr 2016 10:14:47 -0400 Date: Wed, 13 Apr 2016 14:14:00 -0000 From: Marek Polacek To: GCC Patches , Joseph Myers Subject: C/C++ PATCH to add -Wdangling-else option Message-ID: <20160413141444.GT28445@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) X-SW-Source: 2016-04/txt/msg00584.txt.bz2 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 * 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 *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 %"); 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" " %"); 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