From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47983 invoked by alias); 7 Dec 2019 14:45:49 -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 44913 invoked by uid 89); 7 Dec 2019 14:45:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.1 spammy=predicted, presentation, toplevc, separation X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 07 Dec 2019 14:45:38 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1575729931; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Os9tI4MtR5z74A3pISRSo3HJDALqwNyNfYXyKunseZ4=; b=LfWXjmfB3pYpzMPlxana1QvROF6hCwUeK2h81sesqhHsVCdhijDjYfW4oyQKESVSnpp3VV +bB0YXCj3m2FhkvC1ppCxgGvN2NZNJUuYXjU3N71qRvqCN1nhE+0+tq2i7mcSMTW5hcSGm 73rrhROdIWDsXBX8HhOB/keqoN2p/LM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-209-F1Ag9NkkNUG-v8fEs4zIUg-1; Sat, 07 Dec 2019 09:45:29 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8864F8017DF for ; Sat, 7 Dec 2019 14:45:28 +0000 (UTC) Received: from ovpn-112-27.phx2.redhat.com (ovpn-112-27.phx2.redhat.com [10.3.112.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id F2F9960192; Sat, 7 Dec 2019 14:45:27 +0000 (UTC) Message-ID: Subject: Re: [PATCH 12/49] Add diagnostic paths From: Jeff Law Reply-To: law@redhat.com To: David Malcolm , gcc-patches@gcc.gnu.org Date: Sat, 07 Dec 2019 14:45:00 -0000 In-Reply-To: <1573867416-55618-13-git-send-email-dmalcolm@redhat.com> References: <1573867416-55618-1-git-send-email-dmalcolm@redhat.com> <1573867416-55618-13-git-send-email-dmalcolm@redhat.com> User-Agent: Evolution 3.34.2 (3.34.2-1.fc31) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-12/txt/msg00496.txt.bz2 On Fri, 2019-11-15 at 20:22 -0500, David Malcolm wrote: > This patch adds support for associating a "diagnostic_path" with a > diagnostic: a sequence of events predicted by the compiler that leads > to > the problem occurring, with their locations in the user's source, > text descriptions, and stack information (for handling > interprocedural > paths). > > For example, the following (hypothetical) error has a 3-event > intraprocedural path: > > test.c: In function 'demo': > test.c:29:5: error: passing NULL as argument 1 to 'PyList_Append' > which > requires a non-NULL parameter > 29 | PyList_Append(list, item); > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > 'demo': events 1-3 > | > | 25 | list = PyList_New(0); > | | ^~~~~~~~~~~~~ > | | | > | | (1) when 'PyList_New' fails, returning NULL > | 26 | > | 27 | for (i = 0; i < count; i++) { > | | ~~~ > | | | > | | (2) when 'i < count' > | 28 | item = PyLong_FromLong(random()); > | 29 | PyList_Append(list, item); > | | ~~~~~~~~~~~~~~~~~~~~~~~~~ > | | | > | | (3) when calling 'PyList_Append', passing NULL from > (1) as argument 1 > | > > The patch adds a new "%@" format code for printing event IDs, so that > in the above, the description of event (3) mentions event (1), > showing > the user where the bogus NULL value comes from (the event IDs are > colorized to draw the user's attention to them). > > There is a separation between data vs presentation: the above shows > how > the diagnostic-printing code has consolidated the path into a single > run > of events, since all the events are near each other and within the > same > function; more complicated examples (such as interprocedural paths) > might be printed as multiple runs of events. > > Examples of how interprocedural paths are printed can be seen in the > test suite (which uses a plugin to exercise the code without relying > on specific warnings using this functionality). > > Other output formats include > - JSON, > - printing each event as a separate "note", and > - to not emit paths. > > (I have a separate script that can generate HTML from the JSON, but > HTML > is not my speciality; help from a web front-end expert to make it > look > good would be appreciated). > > gcc/ChangeLog: > * Makefile.in (OBJS): Add tree-diagnostic-path.o. > * common.opt (fdiagnostics-path-format=): New option. > (diagnostic_path_format): New enum. > (fdiagnostics-show-path-depths): New option. > * coretypes.h (diagnostic_event_id_t): New forward decl. > * diagnostic-color.c (color_dict): Add "path". > * diagnostic-event-id.h: New file. > * diagnostic-format-json.cc (json_from_expanded_location): Make > non-static. > (json_end_diagnostic): Call context->make_json_for_path if it > exists and the diagnostic has a path. > (diagnostic_output_format_init): Clear context->print_path. > * diagnostic-path.h: New file. > * diagnostic-show-locus.c (colorizer::set_range): Special-case > when printing a run of events in a diagnostic_path so that they > all get the same color. > (layout::m_diagnostic_path_p): New field. > (layout::layout): Initialize it. > (layout::print_any_labels): Don't colorize the label text for > an > event in a diagnostic_path. > (gcc_rich_location::add_location_if_nearby): Add > "restrict_to_current_line_spans" and "label" params. Pass the > former to layout.maybe_add_location_range; pass the latter > when calling add_range. > * diagnostic.c: Include "diagnostic-path.h". > (diagnostic_initialize): Initialize context->path_format and > context->show_path_depths. > (diagnostic_show_any_path): New function. > (diagnostic_path::interprocedural_p): New function. > (diagnostic_report_diagnostic): Call diagnostic_show_any_path. > (simple_diagnostic_path::num_events): New function. > (simple_diagnostic_path::get_event): New function. > (simple_diagnostic_path::add_event): New function. > (simple_diagnostic_event::simple_diagnostic_event): New ctor. > (simple_diagnostic_event::~simple_diagnostic_event): New dtor. > (debug): New overload taking a diagnostic_path *. > * diagnostic.def (DK_DIAGNOSTIC_PATH): New. > * diagnostic.h (enum diagnostic_path_format): New enum. > (json::value): New forward decl. > (diagnostic_context::path_format): New field. > (diagnostic_context::show_path_depths): New field. > (diagnostic_context::print_path): New callback field. > (diagnostic_context::make_json_for_path): New callback field. > (diagnostic_show_any_path): New decl. > (json_from_expanded_location): New decl. > * doc/invoke.texi (-fdiagnostics-path-format=): New option. > (-fdiagnostics-show-path-depths): New option. > (-fdiagnostics-color): Add "path" to description of default > GCC_COLORS; describe it. > (-fdiagnostics-format=json): Document how diagnostic paths are > represented in the JSON output format. > * gcc-rich-location.h > (gcc_rich_location::add_location_if_nearby): > Add optional params "restrict_to_current_line_spans" and > "label". > * opts.c (common_handle_option): Handle > OPT_fdiagnostics_path_format_ and > OPT_fdiagnostics_show_path_depths. > * pretty-print.c: Include "diagnostic-event-id.h". > (pp_format): Implement "%@" format code for printing > diagnostic_event_id_t *. > (selftest::test_pp_format): Add tests for "%@". > * selftest-run-tests.c (selftest::run_tests): Call > selftest::tree_diagnostic_path_cc_tests. > * selftest.h (selftest::tree_diagnostic_path_cc_tests): New > decl. > * toplev.c (general_init): Initialize global_dc->path_format > and > global_dc->show_path_depths. > * tree-diagnostic-path.cc: New file. > * tree-diagnostic.c (maybe_unwind_expanded_macro_loc): Make > non-static. Drop "diagnostic" param in favor of storing the > original value of "where" and re-using it. > (virt_loc_aware_diagnostic_finalizer): Update for dropped param > of > maybe_unwind_expanded_macro_loc. > (tree_diagnostics_defaults): Initialize context->print_path and > context->make_json_for_path. > * tree-diagnostic.h (default_tree_diagnostic_path_printer): New > decl. > (default_tree_make_json_for_path): New decl. > (maybe_unwind_expanded_macro_loc): New decl. > > gcc/c-family/ChangeLog: > * c-format.c (local_event_ptr_node): New. > (PP_FORMAT_CHAR_TABLE): Add entry for "%@". > (init_dynamic_diag_info): Initialize local_event_ptr_node. > * c-format.h (T_EVENT_PTR): New define. > > gcc/testsuite/ChangeLog: > * gcc.dg/format/gcc_diag-10.c (diagnostic_event_id_t): New > typedef. > (test_diag): Add coverage of "%@". > * gcc.dg/plugin/diagnostic-path-format-default.c: New test. > * gcc.dg/plugin/diagnostic-path-format-inline-events-1.c: New > test. > * gcc.dg/plugin/diagnostic-path-format-inline-events-2.c: New > test. > * gcc.dg/plugin/diagnostic-path-format-inline-events-3.c: New > test. > * gcc.dg/plugin/diagnostic-path-format-none.c: New test. > * gcc.dg/plugin/diagnostic-test-paths-1.c: New test. > * gcc.dg/plugin/diagnostic-test-paths-2.c: New test. > * gcc.dg/plugin/diagnostic-test-paths-3.c: New test. > * gcc.dg/plugin/diagnostic_plugin_test_paths.c: New. > * gcc.dg/plugin/plugin.exp: Add the new plugin and test cases. > > libcpp/ChangeLog: > * include/line-map.h (class diagnostic_path): New forward decl. > (rich_location::get_path): New accessor. > (rich_location::set_path): New function. > (rich_location::m_path): New field. > * line-map.c (rich_location::rich_location): Initialize m_path. Seems like we could make an argument to include this in an expanded "diagnostic framework" maintainership which would allow self-approval. jeff