From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11617 invoked by alias); 25 May 2011 19:22:35 -0000 Received: (qmail 11589 invoked by uid 22791); 25 May 2011 19:22:31 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,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; Wed, 25 May 2011 19:22:14 +0000 Received: (qmail 7773 invoked from network); 25 May 2011 19:22:10 -0000 Received: from unknown (HELO codesourcery.com) (froydnj@127.0.0.2) by mail.codesourcery.com with ESMTPA; 25 May 2011 19:22:10 -0000 Date: Wed, 25 May 2011 19:57:00 -0000 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: <20110525192206.GA29722@nightcrawler> References: <1268945454.2219.13.camel@janis-laptop> <20110522193335.GA9729@bromo.med.uc.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-05/txt/msg01918.txt.bz2 On Sun, May 22, 2011 at 03:25:41PM -0700, H.J. Lu wrote: > FWIW, I tried Janis's patch on 4.6 branch and I got > > /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C: In > function 'void e1()':^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C:29:11: > error: redeclaration of 'int k'^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C:27:12: > error: 'int k' previously declared here^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C: In > function 'void e4()':^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C:63:11: > error: redeclaration of 'int i'^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.dg/parse/pr18770.C:61:14: > error: 'int i' previously declared here^M > > FAIL: g++.dg/parse/pr18770.C prev (test for errors, line 14) > FAIL: g++.dg/parse/pr18770.C redecl (test for errors, line 17) > PASS: g++.dg/parse/pr18770.C prev (test for errors, line 27) > PASS: g++.dg/parse/pr18770.C redecl (test for errors, line 29) > FAIL: g++.dg/parse/pr18770.C prev (test for errors, line 37) > FAIL: g++.dg/parse/pr18770.C redecl (test for errors, line 39) > FAIL: g++.dg/parse/pr18770.C prev (test for errors, line 47) > FAIL: g++.dg/parse/pr18770.C redecl (test for errors, line 53) > PASS: g++.dg/parse/pr18770.C prev (test for errors, line 61) > PASS: g++.dg/parse/pr18770.C redecl (test for errors, line 63) > FAIL: g++.dg/parse/pr18770.C prev (test for errors, line 71) > FAIL: g++.dg/parse/pr18770.C redecl (test for errors, line 73) > PASS: g++.dg/parse/pr18770.C (test for excess errors) > > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C: > In function 'int main()':^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:22:11: > error: redeclaration of 'int i'^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:20:14: > error: 'int i' previously declared here^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:27:11: > error: redeclaration of 'int i'^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:25:14: > error: 'int i' previously declared here^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:36:16: > error: types may not be defined in conditions^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:39:3: > error: 'A' was not declared in this scope^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:39:5: > error: expected ';' before 'bar'^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:42:12: > error: types may not be defined in conditions^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:42:40: > error: 'one' was not declared in this scope^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:51:14: > warning: declaration of 'int f()' has 'extern' and is initialized > [enabled by default]^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:51:18: > error: function 'int f()' is initialized like a variable^M > /export/gnu/import/git/gcc/gcc/testsuite/g++.old-deja/g++.jason/cond.C:55:23: > error: extended initializer lists only available with -std=c++0x or > -std=gnu++0x^M > > FAIL: g++.old-deja/g++.jason/cond.C (test for errors, line 9) > FAIL: g++.old-deja/g++.jason/cond.C (test for errors, line 11) > FAIL: g++.old-deja/g++.jason/cond.C (test for errors, line 16) > PASS: g++.old-deja/g++.jason/cond.C (test for errors, line 20) > PASS: g++.old-deja/g++.jason/cond.C (test for errors, line 22) > PASS: g++.old-deja/g++.jason/cond.C (test for errors, line 25) > PASS: g++.old-deja/g++.jason/cond.C (test for errors, line 27) > FAIL: g++.old-deja/g++.jason/cond.C (test for errors, line 30) > FAIL: g++.old-deja/g++.jason/cond.C (test for errors, line 33) > PASS: g++.old-deja/g++.jason/cond.C (test for errors, line 36) > PASS: g++.old-deja/g++.jason/cond.C decl (test for errors, line 39) > PASS: g++.old-deja/g++.jason/cond.C exp (test for errors, line 39) > PASS: g++.old-deja/g++.jason/cond.C def (test for errors, line 42) > PASS: g++.old-deja/g++.jason/cond.C expected (test for errors, line 42) > PASS: g++.old-deja/g++.jason/cond.C extern (test for warnings, line 51) > > The patch no longer catches all problems. 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. The new logic of the patch claims that: int i; for (int i : a) { int i; } is incorrect (the innermost `i' is an erroneous redeclaration). If you apply the expansion of range-based for loops from [stmt.ranged]p1, you'd get something like: for (...; ...; ...) { int i = ...; int i; } which is bad. I believe [basic.scope.local]p4 says much the same thing. Tested with g++ testsuite on x86_64-unknown-linux-gnu; tests in progress for libstdc++. OK to commit? -Nathan 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..2fe4702 100644 --- a/gcc/testsuite/g++.dg/cpp0x/range-for5.C +++ b/gcc/testsuite/g++.dg/cpp0x/range-for5.C @@ -49,6 +49,6 @@ void test1() int i; for (int i : a) { - 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" }