From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id BAFFF3858D35 for ; Wed, 29 Jul 2020 08:40:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BAFFF3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=richard.sandiford@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4AAD231B; Wed, 29 Jul 2020 01:40:37 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CB3FB3F71F; Wed, 29 Jul 2020 01:40:36 -0700 (PDT) From: Richard Sandiford To: Marek Polacek via Gcc-patches Mail-Followup-To: Marek Polacek via Gcc-patches , Marek Polacek , richard.sandiford@arm.com Cc: Marek Polacek Subject: Re: RFC: Monitoring old PRs, new dg directives References: <20200728214447.GS3841@redhat.com> Date: Wed, 29 Jul 2020 09:40:35 +0100 In-Reply-To: <20200728214447.GS3841@redhat.com> (Marek Polacek via Gcc-patches's message of "Tue, 28 Jul 2020 17:44:47 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, 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: Wed, 29 Jul 2020 08:40:39 -0000 Thanks for doing this. +1 for the best fix being to add XFAILing tests to the main testsute, enabled by default. I don't see any other realistic way of ensuring that fixes are matched with PRs at the time that the fix is made (rather than some time after the fact). Marek Polacek via Gcc-patches writes: > [=E2=80=A6] > My thinking is that for: > > * rejects-valid: use the existing dg-xfail-if > * accepts-valid: use the new dg-accepts-invalid > * ICEs: use the new dg-ice > > dg-ice can be used like this: > > // { dg-ice "build_over_call" { target c++11 } } > > and it means that if the test still ICEs, you'll get a quiet XFAIL. If t= he > ICE is fixed, you'll get an XPASS; if the ICE is gone but there are error= s, > you'll get an XPASS + FAIL. Then you can close the old PR. This is long overdue IMO, thanks for adding it. > Similarly, dg-accepts-invalid: > > // { dg-accepts-invalid "PR86500" } > > means that if the test still compiles without errors, you'll get a quiet = XFAIL. > If we start giving errors, you'll get an XPASS. > > If the bug is fixed, simply remove the directive. > > The patch implementing these new directives is appended. Once/if this is > accepted, I can start adding the old tests we have in our Bugzilla. (I'm > only concerned about the c++ component, if that wasn't already clear.) > > The question is what makes the bug "old": is it one year without it being > assigned? 6 months? 3 months? Note: I *don't* propose to add every tes= t for > every new PR, just the reasonably old ones that are useful/important. Su= ch > additions should be done in batches, so that we don't have dozens of comm= its, > each of them merely adding a single test. IMO it should be OK to add a testcase for any open PR, if someone think it's useful, regardless of age and without being forced to batch the commits. I.e. I think it should come under the =E2=80=9Cobvious=E2=80=9D r= ule and people should just use their judgement about when it's appropriate. Adding XFAILing tests shouldn't disturb anyone else very much. I guess there's a possibility that some tests happen to pass already on some targets. That's more likely with middle-end and backend bugs rather than frontend stuff though. Perhaps for those it would make sense to have a convention in which the failing testcase is restricted (at the whole-test level) to the targets that the person committing the testcase has actually tried. Maybe with a comment on the dg-ice etc. to remind people to reconsider the main target selector when un-XFAILing the test. Richard