From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk1-xa29.google.com (mail-vk1-xa29.google.com [IPv6:2607:f8b0:4864:20::a29]) by sourceware.org (Postfix) with ESMTPS id 69023385BF81; Thu, 9 Apr 2020 20:59:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 69023385BF81 Received: by mail-vk1-xa29.google.com with SMTP id m131so90599vkh.3; Thu, 09 Apr 2020 13:59:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=iI0djXOwlVlKQHl6HZCW2+n9INK5ba0kOSo2SBAYKiY=; b=bzuKed59dN/ZrVbAjtlEfGxqoIG0PmREPTmXjFKnFIPeg2nDhvC3T1Z8dnyo4qsZsi IEp7c20xWO2fOi+jjmvc53NikOWCMmEGjuRI0+Dvuxv2OcZB6nBRzi9rbObedPFWuidA WXnIuoG/3f8mb2HBg3GQvOhhq7/OJZFajvdnNS0lhHuL/IpjazzgXtd0Z4cG2yJGApFZ Bn/7/qtaSYMxJdNfyauaO86UuE4OH9l59UspXTQKWjm/lfKg8tnsuGKqUBxwoywJy6VV 607y4y/KqgfvkZYMqzTt83q1Jjxm2ywp45FT5XLG9Pj26blc1hzyAQIv02G23FmluVXr qXOg== X-Gm-Message-State: AGi0PuaMsevHlgfcOulOigp3ciX+HsrjSqxWs91DsgXbDxdYuDNft7wI sezd98A3nhGfP5AsJ4cgaQIcuRRVvz3RzV/v5Kcdw5kA X-Google-Smtp-Source: APiQypJsQ1RUFx+pP+WmZp3oVHbZLTbd/5L1NG6T6UAdaMXcdlnmBdbqbqf6mIxxCep3+0cyoYWMY+/0kiEjNDN9WpA= X-Received: by 2002:a1f:a644:: with SMTP id p65mr1331467vke.74.1586465988872; Thu, 09 Apr 2020 13:59:48 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Fritz Reese Date: Thu, 9 Apr 2020 16:59:37 -0400 Message-ID: Subject: Re: [PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving I/O tags and simplify io.c To: Tobias Burnus Cc: fortran , gcc-patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_2, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Apr 2020 20:59:52 -0000 Tobias, Thanks very much for the review. On Thu, Apr 9, 2020 at 5:21 AM Tobias Burnus wrot= e: > > Hi, > > On 4/6/20 7:25 PM, Fritz Reese via Fortran wrote: > > > The attached patch fixes PR 87923 while also simplifying the code in > > io.c. > > Thanks for the work, which looks great; it is a bit lengthy > but mostly moving code or mechanical (%C =E2=86=92 %L). > It also has an impressive amount of new test cases! I also wished the patch could be easier on the eyes, but alas sometimes this is the price of progress. :-) > * First line in git commit "Fix fortran/87923 -- ICE(s) when resolving I/= O tags." > It helps with doing patch archeology if they are the same =E2=80=93 or= if the GIT one > is a substring of the email subject. (If it is about a PR, searching f= or the PR > usually works, but also not al emails have the PR number in the subjec= t.) > For this patch, you use: > email: "[PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving= I/O tags and simplify io.c" > GIT: "Fix fortran/87923 -- ICE(s) when resolving I/O tags." Yes, that is a good point. I will alter the commit summary to match the email subject. > * Please check whether the ChangeLog lines are too long; I didn't count > but it looks as if they might be too long. For sure, they > were too long for your mail program =E2=80=A6 I copied the git commit message from the log, which git formats with an extra level of indentation. I wrapped the raw ChangeLog entries and commit message to 80 characters, but after the extra indentation my mail client indeed wrapped the lines during post-processing. I suppose I should wrap these each to 76 to account for git's indentation. That certainly makes "git log" look better. > * In the following comment, you have two empty tailing lines: > > + Tag expressions are already resolved by resolve_tag, which includes > + verifying the type, that they are scalar, and verifying that BT_CHARA= CTER > + tags are of default kind. > + > + */ Oops! I will commit the patch with these fixes rebased on master after one final build+test. Thanks again for taking a look. Cheers, --- Fritz Reese