From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 68663 invoked by alias); 15 Sep 2015 00:53:57 -0000 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 Received: (qmail 68346 invoked by uid 89); 15 Sep 2015 00:53:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL,BAYES_40,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no 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, 15 Sep 2015 00:53:55 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 95BD72630 for ; Tue, 15 Sep 2015 00:53:53 +0000 (UTC) Received: from [10.3.224.166] (vpn-224-166.phx2.redhat.com [10.3.224.166]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8F0rq27003919; Mon, 14 Sep 2015 20:53:52 -0400 Message-ID: <1442278432.32352.87.camel@surprise> Subject: Re: [PATCH 00/22] RFC: Overhaul of diagnostics From: David Malcolm To: Jeff Law Cc: Bernd Schmidt , gcc-patches@gcc.gnu.org Date: Tue, 15 Sep 2015 01:11:00 -0000 In-Reply-To: <55F7233A.3010603@redhat.com> References: <1441916913-11547-1-git-send-email-dmalcolm@redhat.com> <55F7073E.8000308@redhat.com> <55F7233A.3010603@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00992.txt.bz2 On Mon, 2015-09-14 at 13:42 -0600, Jeff Law wrote: > On 09/14/2015 11:43 AM, Bernd Schmidt wrote: > > It's hard to provide meaningful review under these conditions. My advice > > would be to resubmit the things that are ready now and can stand on > > their own so that we can get them out of the way first. Also, gather > > memory/time information before posting the patches if that seems likely > > to be important. For example, patch 21 looks quite cool but also > > potentially expensive, I'd probably want that to be restricted by param > > to identifiers of a maximum length (for both identifiers being compared). > I think David is looking for some feedback on some of this stuff. > There's clearly some design/implementation issues in those middling > patches. The thought behind showing the later patches is so that folks > can generally see where this work is trying to go. Indeed: my hope was that it would be helpful to see the kinds of diagnostics I was hoping to be able to print, as that motivates both the changes to diagnostics_show_locus, and efforts to try to capture and store range information somehow within our IR. I can post more screenshots if it will be helpful. > One of my big worries is the memory consumption. Yes. Clearly the implementation I have in patch 12 isn't going to fly; ideas welcome. One thing I may try next is to only try to track the ranges as the trees are constructed, immediately discarding them once we've done that first level of error-checking... basically to not store it beyond the frontends (for example to stuff it into c_expr in the C FE). That might be a useful compromise: hopefully letting us make a lot of diagnostics more readable, without bloating the memory requirements. That's my hope, anyway :) > > For the most part I declare myself agnostic as to whether this is an > > improvement or not, and leave that for others to comment on. I > > personally prefer single-line errors without much noise. > I wasn't a fan of rich location diagnostics, carets, etc. However, now > that I'm doing more C++ bits, I'm seeing the utility of this kind of stuff. FWIW, I've mostly been holding off on adding ranges to the C++ FE in the hope that the delayed folding branch will get merged soon (since otherwise its unclear what to base the changes on); hence I only touched a few places where token ranges were in use; I didn't attempt tree ranges. > > I see lots of unit tests implemented as plugins - have we decided that > > this is the mechanism we want to use for this kind of thing? > A lot of the plugin-based testing is stuff that's painful to test > end-to-end. Probably the best way to think of those tests is they're > trying to directly test internal state. Right. The new plugins allow us to exercise the underlying machinery unit by unit, and this is good for sanity (in particular, mine). The unit tests in this patch kit use source code, which is going to be the case for some tests, and fits neatly into the gcc.dg/plugin/plugin.exp pattern, but not every test fits this pattern. By contrast, if we want to e.g. verify that gengtype generates sane mark&sweep routines, that doesn't necessarily need specific source code. This latter style of test is what I was thinking of in the other patch kit I posted here: https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00765.html ("[PATCH 00/17] RFC: Adding a unit testing framework to gcc"). It's kind of a pain to write a plugin each time we poke at a data structure, and run it with an empty source file, so my thinking was to consolidate those tests that simply exercise internal data structures into a single unit-test plugin, and run all the tests within it. In particular, my hope is that this style of test could (a) help us track down bugs earlier [1] and (b) be dramatically faster: I want us to be measuring e.g. how many 100s or 1000s of unit tests per second we can run, rather than having to fork/exec subprocesses for just a few tests each time. (Though that's probably a different discussion). Thanks for the comments. Hope the above sounds sane. Dave [1] I *hate* tracking down gengtype bugs; I'm keen to give us direct test coverage for the code it generates, so we can track down bugs immediately, rather than with multi-hour gdb sessions...