From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26914 invoked by alias); 19 Aug 2014 21:47:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 26901 invoked by uid 89); 19 Aug 2014 21:47:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 19 Aug 2014 21:47:26 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7JLlNVm023316 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 19 Aug 2014 17:47:23 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s7JLlLJ5015545; Tue, 19 Aug 2014 17:47:22 -0400 Message-ID: <53F3C5E9.4020000@redhat.com> Date: Tue, 19 Aug 2014 21:47:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Doug Evans , GDB Patches Subject: Re: [RFC] delete gdb.cp/ambiguous.exp ? References: <53F38030.1020406@redhat.com> <53F38E62.60403@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-08/txt/msg00362.txt.bz2 On 08/19/2014 09:10 PM, Doug Evans wrote: > On Tue, Aug 19, 2014 at 10:50 AM, Pedro Alves wrote: >> On 08/19/2014 06:24 PM, Doug Evans wrote: >> >>>> Actually enabling the test (removing the skip, and adding >>>> nowarnings), we see that indeed GDB outputs no warning: >>> >>> But given the early exit the test itself is never run. >> >> As I said, I removed that. ;-) > > I know (I thought that was obvious). The statement you replied to clearly said I locally enabled the test (_that_ much seems obvious to me), and you replied back as if I hadn't. It just sounded like the start of going in circles. So I still don't know what did you intend to actually say or refute to. >>> I'm all for filing a bug and recording the test in the bug report. >>> I'm even ok with keeping the test as is. >>> The high order bit for me is exploring what the community wishes be >>> done with this kind of "dead code". >> >> You're asking for a generic opinion, of the abstract "community", >> while I think "case by case" generally applies. ;-) > > I'm asking people for their opinion on this case (and cases like it). > If I call them a "community", which is what gdb@ and gdb-patches@ is > to me, I don't see the problem. [ This is getting off topic, and about how we can improve communication [which I am all for!], but I think the problem for me is in the very lose definition of what is a "this case", and then wanting a generic rule. I don't know what you mean by that exactly. It begs for a generic "thou shalt do this or that" kind of answer, but, personally, I find it difficult to answer such a question. I think we should be reasonable and look at things in a little more detail than applying a rubber stamp rule. So I did my best to actually look at the test and give out a rationale for why I thought _enabling_ the test (because it's skipped today) would be more useful than deleting it under a broad "dead code" rule. The answer I got back, in turn was, paraphrasing: "yeah, but never runs today or for a long while!". Well, I know that, of course, otherwise I wouldn't have gone through the trouble of force-enabling the test locally! Again, that feels like a discussion that will quickly degenerate to going around in circles. It feels like we tend to talk past one another frequently, while I believe there's no intention to on either side. How can we improve here? ] > > Removal of dead code is, in my experience here, *rarely* a case by > case decision. > [each case may have minor details that are different, but the high > order bit is generally DELETE IT :-)] Right. When I say "case by case", I'm kind of defending against considering unreachable C code the same as #if 0 code that is serving as comment the same as clearly dead testsuite/lib/*.exp code the same as a test that happens to be skipped on the most popular platform. I don't think it's so rare that an investigation finds that what is currently dead could or should be reachable and a fix in that direction is made. And we don't have to go very far for that: https://sourceware.org/ml/gdb-patches/2014-08/msg00180.html > And yet this is a little different than an #if 0 in code, so I'm asking. Yeah. > We certainly want to document this issue in some way, but if we're > just going to leave the test as is, not being run, and yet have to > disable it for new compilers as they come along, then I think there's > value in documenting the issue in a different way (e.g., bug reports > are fine for this sort of thing). > >> My inclination for >> tests is to first check whether there's something salvageable. If >> someone wrote the test, it was probably important enough. If it's indeed >> "dead code", then of I'd go for just removing it. I looked, and it seemed >> to me that the test actually covers an aspect of printing that we don't >> cover elsewhere, and actually reveals a bug. >> So IMO, in this particular case, we should keep the test, remove the gcc check, >> modernize and KFAIL it , and then have the bug fixed (if people agree it's >> a bug). I'm of course not suggesting you do that all of yourself, but >> since you asked, that's what I'd prefer see done in this case. > > This is feedback I can work with. Thanks. > > Going forward, as we discover bugs, are people ok with checking in a > KFAIL'd test for a bug before the bug is fixed? It depends on the bug, but in general yes I'm okay with KFAILs. I've added and fixed some myself. > "Just fix the bug." > is the canonical response. Again a generalization. ;-) Myself, I'll give out that response to workarounds when I think the fix is doable in reasonable time compared to the time needed to do the workaround. I may be mistaken, but I don't recall saying that in response to a new KFAIL test, unless again, I trust the fix is more trivial than the submitter initially believes. > And yet there are often higher priorities. > How do we document the bug's existence so that it's not forgotten > until someone has time to fix it? File a bug report, of course. > Well, I file bug reports all the time, I certainly don't have time to > fix all of them instead of filing the bug report. And if in > reproducing the bug I'm 80% of the way there in writing the test, is > it ok to at least finish and check in that part, even if I don't have > time to fix the bug? IMO, yes, within reason. > Or, instead of allowing that, should we just "grandfather in", so to > speak, all existing disabled tests, not allow new KFAIL'd ones, and > migrate these existing tests to KFAIL if we don't have time to fix the > bug? I don't think we should forbid new KFAILed tests as a rule. Thanks, Pedro Alves