From patchwork Thu May 26 01:24:28 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Froyd X-Patchwork-Id: 97482 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id C269AB6F8C for ; Thu, 26 May 2011 11:24:54 +1000 (EST) Received: (qmail 25756 invoked by alias); 26 May 2011 01:24:51 -0000 Received: (qmail 25745 invoked by uid 22791); 26 May 2011 01:24:50 -0000 X-SWARE-Spam-Status: No, hits=-0.3 required=5.0 tests=AWL, BAYES_50, TW_CX, TW_TM, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 26 May 2011 01:24:34 +0000 Received: (qmail 29117 invoked from network); 26 May 2011 01:24:31 -0000 Received: from unknown (HELO codesourcery.com) (froydnj@127.0.0.2) by mail.codesourcery.com with ESMTPA; 26 May 2011 01:24:31 -0000 Date: Wed, 25 May 2011 21:24:28 -0400 From: Nathan Froyd To: "H.J. Lu" Cc: Gerald Pfeifer , Jack Howarth , Janis Johnson , gcc-patches@gcc.gnu.org, Richard Guenther , jason@redhat.com Subject: Re: [PATCH][4.6] detect C++ errors to fix 2288 and 18770 Message-ID: <20110526012420.GA15460@nightcrawler> References: <1268945454.2219.13.camel@janis-laptop> <20110522193335.GA9729@bromo.med.uc.edu> <20110525192206.GA29722@nightcrawler> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20110525192206.GA29722@nightcrawler> User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Wed, May 25, 2011 at 03:22:07PM -0400, Nathan Froyd wrote: > The patch just requires some shuffling of logic to catch issues now; > below is a version that works for me on the trunk. > > This new checking does require modifying g++.dg/cpp0x/range-for5.C. > > Tested with g++ testsuite on x86_64-unknown-linux-gnu; tests in progress > for libstdc++. OK to commit? Below is a slightly revised patch that actually adds all the necessary dg-error directives to range-for5.C. Tested the same way; Peter Bergner was kind enough to bootstrap and test the previous patch on powerpc64-linux-gnu with (almost) clean results as well. gcc/cp/ 2011-xx-xx Janis Johnson Nathan Froyd PR c++/2288 PR c++/18770 * name-lookup.h (enum scope_kind): Add sk_cond. * name-lookup.c (pushdecl_maybe_friend): Get scope of shadowed local. Detect and report error for redeclaration from for-init or if or switch condition. (begin_scope): Handle sk_cond. * semantics.c (begin_if_stmt): Use sk_cond. (begin switch_stmt): Ditto. gcc/testsuite/ 2011-xx-xx Janis Johnson Nathan Froyd PR c++/2288 PR c++/18770 * g++.old-deja/g++.jason/cond.C: Remove xfails. * g++.dg/parse/pr18770.C: New test. * g++.dg/cpp0x/range-for5.C: Add dg-error marker. diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index bb6d4b9..723f36f 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -957,8 +957,15 @@ pushdecl_maybe_friend_1 (tree x, bool is_friend) else { /* Here to install a non-global value. */ - tree oldlocal = innermost_non_namespace_value (name); tree oldglobal = IDENTIFIER_NAMESPACE_VALUE (name); + tree oldlocal = NULL_TREE; + cxx_scope *oldscope = NULL; + cxx_binding *oldbinding = outer_binding (name, NULL, true); + if (oldbinding) + { + oldlocal = oldbinding->value; + oldscope = oldbinding->scope; + } if (need_new_binding) { @@ -1087,6 +1094,20 @@ pushdecl_maybe_friend_1 (tree x, bool is_friend) } } } + /* Error if redeclaring a local declared in a + for-init-statement or in the condition of an if or + switch statement when the new declaration is in the + outermost block of the controlled statement. + Redeclaring a variable from a for or while condition is + detected elsewhere. */ + else if (TREE_CODE (oldlocal) == VAR_DECL + && oldscope == current_binding_level->level_chain + && (oldscope->kind == sk_cond + || oldscope->kind == sk_for)) + { + error ("redeclaration of %q#D", x); + error ("%q+#D previously declared here", oldlocal); + } if (warn_shadow && !nowarn) { @@ -1446,6 +1467,7 @@ begin_scope (scope_kind kind, tree entity) case sk_try: case sk_catch: case sk_for: + case sk_cond: case sk_class: case sk_scoped_enum: case sk_function_parms: diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h index 4bf253f..90b56e4 100644 --- a/gcc/cp/name-lookup.h +++ b/gcc/cp/name-lookup.h @@ -109,6 +109,8 @@ typedef enum scope_kind { sk_catch, /* A catch-block. */ sk_for, /* The scope of the variable declared in a for-init-statement. */ + sk_cond, /* The scope of the variable declared in the condition + of an if or switch statement. */ sk_function_parms, /* The scope containing function parameters. */ sk_class, /* The scope containing the members of a class. */ sk_scoped_enum, /* The scope containing the enumertors of a C++0x diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 55ad117..7833d76 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -656,7 +656,7 @@ tree begin_if_stmt (void) { tree r, scope; - scope = do_pushlevel (sk_block); + scope = do_pushlevel (sk_cond); r = build_stmt (input_location, IF_STMT, NULL_TREE, NULL_TREE, NULL_TREE, scope); begin_cond (&IF_COND (r)); @@ -1013,7 +1013,7 @@ begin_switch_stmt (void) { tree r, scope; - scope = do_pushlevel (sk_block); + scope = do_pushlevel (sk_cond); r = build_stmt (input_location, SWITCH_STMT, NULL_TREE, NULL_TREE, NULL_TREE, scope); begin_cond (&SWITCH_STMT_COND (r)); diff --git a/gcc/testsuite/g++.dg/cpp0x/range-for5.C b/gcc/testsuite/g++.dg/cpp0x/range-for5.C index 9c97ad5..fd6f761 100644 --- a/gcc/testsuite/g++.dg/cpp0x/range-for5.C +++ b/gcc/testsuite/g++.dg/cpp0x/range-for5.C @@ -47,8 +47,8 @@ void test1() //Check the correct scopes int i; - for (int i : a) + for (int i : a) // { dg-error "previously declared" } { - int i; + int i; // { dg-error "redeclaration" } } } diff --git a/gcc/testsuite/g++.dg/parse/pr18770.C b/gcc/testsuite/g++.dg/parse/pr18770.C new file mode 100644 index 0000000..df57be4 --- /dev/null +++ b/gcc/testsuite/g++.dg/parse/pr18770.C @@ -0,0 +1,175 @@ +/* { dg-do compile } */ + +/* The ISO C++ standard says, in Section 3.3.2 sentence 4, that a name + declared in the for-init-statement or in the condition of an if, for + while, or switch statement can't be redeclared in the outermost block + of the controlled statement. (Note, this is not an error in C.) */ + +extern void foo (int); +extern int j; + +void +e0 (void) +{ + for (int i = 0; // { dg-error "previously declared here" "prev" } + i < 10; ++i) + { + int i = 2; // { dg-error "redeclaration" "redecl" } + foo (i); + } +} + +void +e1 (void) +{ + int i; + for (i = 0; + int k = j; i++) // { dg-error "previously declared here" "prev" } + { + int k = 2; // { dg-error "redeclaration" "redecl" } + foo (k); + } +} + +void +e2 (void) +{ + if (int i = 1) // { dg-error "previously declared here" "prev" } + { + int i = 2; // { dg-error "redeclaration" "redecl" } + foo (i); + } +} + +void +e3 (void) +{ + if (int i = 1) // { dg-error "previously declared here" "prev" } + { + foo (i); + } + else + { + int i = 2; // { dg-error "redeclaration" "redecl" } + foo (i); + } +} + +void +e4 (void) +{ + while (int i = 1) // { dg-error "previously declared here" "prev" } + { + int i = 2; // { dg-error "redeclaration" "redecl" } + foo (i); + } +} + +void +e5 (void) +{ + switch (int i = j) // { dg-error "previously declared here" "prev" } + { + int i; // { dg-error "redeclaration" "redecl" } + default: + { + i = 2; + foo (i); + } + } +} + +void +f0 (void) +{ + for (int i = 0; i < 10; ++i) + { + foo (i); + { + int i = 2; // OK, not outermost block. + foo (i); + } + } +} + +void +f1 (void) +{ + int i; + for (i = 0; int k = j; i++) + { + foo (k); + { + int k = 2; // OK, not outermost block. + foo (k); + } + } +} + +void +f2 (void) +{ + if (int i = 1) + { + foo (i); + { + int i = 2; // OK, not outermost block. + foo (i); + } + } +} + +void +f3 (void) +{ + if (int i = 1) + { + foo (i); + } + else + { + foo (i+2); + { + int i = 2; // OK, not outermost block. + foo (i); + } + } +} + +void +f4 (void) +{ + while (int i = 1) + { + foo (i); + { + int i = 2; // OK, not outermost block. + foo (i); + } + } +} + +void +f5 (void) +{ + switch (int i = j) + { + default: + { + int i = 2; // OK, not outermost block. + foo (i); + } + } +} + +void +f6 (void) +{ + int i = 1; + + for (int j = 0; j < 10; j++) + { + int i = 2; // OK, not variable from for-init. + foo (i); + } +} diff --git a/gcc/testsuite/g++.old-deja/g++.jason/cond.C b/gcc/testsuite/g++.old-deja/g++.jason/cond.C index d0616e4..a6e5ba0 100644 --- a/gcc/testsuite/g++.old-deja/g++.jason/cond.C +++ b/gcc/testsuite/g++.old-deja/g++.jason/cond.C @@ -6,14 +6,14 @@ int main() { float i; - if (int i = 1) // { dg-error "" "" { xfail *-*-* } } , + if (int i = 1) // { dg-error "previously" } { - char i; // { dg-error "" "" { xfail *-*-* } } , + char i; // { dg-error "redeclaration" } char j; } else { - short i; // { dg-error "" "" { xfail *-*-* } } , + short i; // { dg-error "redeclaration" } char j; } @@ -27,10 +27,10 @@ int main() int i; // { dg-error "redeclaration" } } - switch (int i = 0) // { dg-error "" "" { xfail *-*-* } } + switch (int i = 0) // { dg-error "previously" } { default: - int i; // { dg-error "" "" { xfail *-*-* } } + int i; // { dg-error "redeclaration" } } if (struct A { operator int () { return 1; } } *foo = new A) // { dg-error "defined" }