From patchwork Wed May 6 11:38:36 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 468882 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 7D3E2140134 for ; Wed, 6 May 2015 21:39:04 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=aye/AMD7; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; q=dns; s=default; b=Yx/g6XfIY+PW5B4l 3Jha6QTQ4TnPeXmSnEGzTsx0hIF6BX3gmBZd/e6WwUtamz5CazVcpKDni3N4Aszc 0XpZAMkGxb2fefI6dzBaWK+2bzxOFvxOILXZ6cKRqayvRSKvGn6LJlb3vfg1YfWR wbSnZwkzuTDrveqPZ9secIktAEI= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; s=default; bh=AfqvfYGRa5/AmhVxChWCrL +sZO8=; b=aye/AMD7EM2Kn3y83G2RTJaz4feWvGh3E+xwNl8AZG4JqhmmQVUwec MVIuaKmdfLdPxaVUiRbAyBiEGAC/Cm5A9tMnhRb9/GSNzF1SKXFQjdFr5U0kCid/ T5wey4rzOo77IA8Y3xLAl9FGsPk1FOPUP9QtVQ204xaROnRFh0sm4= Received: (qmail 34303 invoked by alias); 6 May 2015 11:38:57 -0000 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 Received: (qmail 34278 invoked by uid 89); 6 May 2015 11:38:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 06 May 2015 11:38:54 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1Ypxf6-0001nJ-4K from Thomas_Schwinge@mentor.com ; Wed, 06 May 2015 04:38:48 -0700 Received: from feldtkeller.schwinge.homeip.net (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Wed, 6 May 2015 12:38:46 +0100 From: Thomas Schwinge To: , Tobias Burnus CC: , David Malcolm , Mikael Morin , Ilmir Usmanov Subject: Fix logic error in Fortran OpenACC parsing (was: [PATCH 3/3] Fix indentation issues seen by -Wmisleading-indentation) In-Reply-To: <1430854683.32584.275.camel@surprise> References: <5536936F.8090004@gmail.com> <1430265776-8157-1-git-send-email-dmalcolm@redhat.com> <1430265776-8157-3-git-send-email-dmalcolm@redhat.com> <5540CA39.7070308@sfr.fr> <1430854683.32584.275.camel@surprise> User-Agent: Notmuch/0.9-101-g81dad07 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) Date: Wed, 6 May 2015 13:38:36 +0200 Message-ID: <87d22endfn.fsf@schwinge.name> MIME-Version: 1.0 Hi! On Tue, 5 May 2015 15:38:03 -0400, David Malcolm wrote: > On Wed, 2015-04-29 at 14:10 +0200, Mikael Morin wrote: > > Le 29/04/2015 02:02, David Malcolm a écrit : > > > diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c > > > index 2c7c554..30e4eab 100644 > > > --- a/gcc/fortran/parse.c > > > +++ b/gcc/fortran/parse.c > > > @@ -4283,7 +4283,7 @@ parse_oacc_structured_block (gfc_statement acc_st) > > > unexpected_eof (); > > > else if (st != acc_end_st) > > > gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st)); > > > - reject_statement (); > > > + reject_statement (); > > > } > > > while (st != acc_end_st); > > > > > I think this one is a bug; there should be braces around 'gfc_error' and > > 'reject_statement'. Mikael, thanks for noticing this, and, David, nice -Wmisleading-indentation patch set! > > At least that's the pattern in 'parse_oacc_loop', and how the > > 'unexpected_statement' function is used. > FWIW, Jeff had approved that patch, so I've committed the patch to trunk > (as r222823), making the indentation reflect the block structure. > > Thomas: should the > reject_statement (); > call in the above be guarded by the > else if (st != acc_end_st) > clause? Indeed, this seems to be a bug that has been introduced very early in the OpenACC Fortran front end development -- see how the parse_oacc_structured_block function evolved in the patches posted in and following (Ilmir, CCed "just in case"). I also see that the corresponding OpenMP code, parse_omp_structured_block, just calls unexpected_statement, which Ilmir's initial patch also did, but at some point, he then changed this to the current code: gfc_error followed by reject_statement, as cited above -- I would guess for the reason to get a better error message? (Tobias, should this thus also be done for OpenMP, and/or extend unexpected_statement accordingly?) And then, I'm a bit confused: is it "OK" that despite this presumed logic error, which affects all (?) valid executions of this parsing code, we're not running into any issues with the OpenACC Fortran front end test cases? OK for trunk? commit 068eebfa63b2b4c8849ed5fd2c9d0a130586dfb0 Author: Thomas Schwinge Date: Wed May 6 13:18:18 2015 +0200 Fix logic error in Fortran OpenACC parsing gcc/fortran/ * parse.c (parse_oacc_structured_block): Fix logic error. Reported by Mikael Morin . --- gcc/fortran/parse.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) Grüße, Thomas diff --git gcc/fortran/parse.c gcc/fortran/parse.c index 30e4eab..e977498 100644 --- gcc/fortran/parse.c +++ gcc/fortran/parse.c @@ -4282,8 +4282,10 @@ parse_oacc_structured_block (gfc_statement acc_st) if (st == ST_NONE) unexpected_eof (); else if (st != acc_end_st) - gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st)); - reject_statement (); + { + gfc_error ("Expecting %s at %C", gfc_ascii_statement (acc_end_st)); + reject_statement (); + } } while (st != acc_end_st);