From 190db66e9a2410aef413abd89264dcb2c59791b8 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Thu, 12 Mar 2026 08:48:31 -0400 Subject: [PATCH 24/38] diagnostics: use label_text in diagnostics::option_id_manager Eliminate some manual memory management. gcc/ChangeLog: * diagnostics/context.h (diagnostics::context::make_option_name): Convert return type from char * to label_text, and rename to... (diagnostics::context::get_option_name): ...this. (diagnostics::context::make_option_url): Likewise, renaming to... (diagnostics::context::get_option_url): ...this. * diagnostics/html-sink.cc (html_builder::make_element_for_diagnostic): Update for above changes. * diagnostics/lazy-paths.cc (selftest::all_warnings_disabled): Update for above changes. * diagnostics/option-id-manager.h (diagnostics::option_id_manager::make_option_name): Convert return type from char * to label_text, and rename to... (diagnostics::option_id_manager::get_option_name): ...this. (diagnostics::option_id_manager::make_option_url): Likewise, renaming to... (diagnostics::option_id_manager::get_option_url): ...this. * diagnostics/sarif-sink.cc: Add #define INCLUDE_SET. (sarif_builder::m_rule_id_set): Convert from hash_set to std::set. (sarif_builder::make_result_object): Update for above changes, removing manual memory management. (sarif_builder::make_reporting_descriptor_object_for_warning): Likewise. * diagnostics/text-sink.cc (text_sink::print_option_information): Likewise. * opts-diagnostic.h (gcc_diagnostic_option_id_manager): Update for above changes. * opts.cc (compiler_diagnostic_option_id_manager::make_option_name): Update as above, renaming to... (compiler_diagnostic_option_id_manager::get_option_name): ...this. (gcc_diagnostic_option_id_manager::make_option_url): Likewise, renaming to... (gcc_diagnostic_option_id_manager::get_option_url): ...this. Signed-off-by: David Malcolm --- gcc/diagnostics/context.h | 20 ++++++------- gcc/diagnostics/html-sink.cc | 12 ++++---- gcc/diagnostics/lazy-paths.cc | 16 +++++++---- gcc/diagnostics/option-id-manager.h | 17 ++++++----- gcc/diagnostics/sarif-sink.cc | 28 +++++++++--------- gcc/diagnostics/text-sink.cc | 25 ++++++++-------- gcc/opts-diagnostic.h | 12 ++++---- gcc/opts.cc | 44 +++++++++++++++-------------- 8 files changed, 88 insertions(+), 86 deletions(-) diff --git a/gcc/diagnostics/context.h b/gcc/diagnostics/context.h index dcfb80760b6..ad1192e93a3 100644 --- a/gcc/diagnostics/context.h +++ b/gcc/diagnostics/context.h @@ -485,22 +485,22 @@ public: return m_option_id_mgr->option_enabled_p (opt_id); } - inline char *make_option_name (option_id opt_id, - enum kind orig_diag_kind, - enum kind diag_kind) const + inline label_text get_option_name (option_id opt_id, + enum kind orig_diag_kind, + enum kind diag_kind) const { if (!m_option_id_mgr) - return nullptr; - return m_option_id_mgr->make_option_name (opt_id, - orig_diag_kind, - diag_kind); + return label_text (); + return m_option_id_mgr->get_option_name (opt_id, + orig_diag_kind, + diag_kind); } - inline char *make_option_url (option_id opt_id) const + inline label_text get_option_url (option_id opt_id) const { if (!m_option_id_mgr) - return nullptr; - return m_option_id_mgr->make_option_url (opt_id); + return label_text (); + return m_option_id_mgr->get_option_url (opt_id); } void diff --git a/gcc/diagnostics/html-sink.cc b/gcc/diagnostics/html-sink.cc index 3a33d73c1e5..cf6c2a106e3 100644 --- a/gcc/diagnostics/html-sink.cc +++ b/gcc/diagnostics/html-sink.cc @@ -1087,13 +1087,13 @@ html_builder::make_element_for_diagnostic (const diagnostic_info &diagnostic, // Add any option as a suffix to the message - label_text option_text = label_text::take - (m_context.make_option_name (diagnostic.m_option_id, - orig_diag_kind, diagnostic.m_kind)); - if (option_text.get ()) + label_text option_text + = m_context.get_option_name (diagnostic.m_option_id, + orig_diag_kind, diagnostic.m_kind); +if (option_text.get ()) { - label_text option_url = label_text::take - (m_context.make_option_url (diagnostic.m_option_id)); + label_text option_url + = m_context.get_option_url (diagnostic.m_option_id); xp.add_text (" "); auto option_span = make_span ("gcc-option"); diff --git a/gcc/diagnostics/lazy-paths.cc b/gcc/diagnostics/lazy-paths.cc index 6d4c676efb3..0320110e360 100644 --- a/gcc/diagnostics/lazy-paths.cc +++ b/gcc/diagnostics/lazy-paths.cc @@ -137,15 +137,19 @@ public: /* Treat all options as disabled. */ return 0; } - char *make_option_name (diagnostics::option_id, - enum kind, - enum kind) const final override + + label_text + get_option_name (diagnostics::option_id, + enum kind, + enum kind) const final override { - return nullptr; + return label_text (); } - char *make_option_url (diagnostics::option_id) const final override + + label_text + get_option_url (diagnostics::option_id) const final override { - return nullptr; + return label_text (); } }; diff --git a/gcc/diagnostics/option-id-manager.h b/gcc/diagnostics/option-id-manager.h index 55c7c429a2b..0d997eecbfa 100644 --- a/gcc/diagnostics/option-id-manager.h +++ b/gcc/diagnostics/option-id-manager.h @@ -35,20 +35,19 @@ public: (or if the value is unknown, typically set later in target). */ virtual int option_enabled_p (option_id opt_id) const = 0; - /* Return malloced memory for the name of the option OPT_ID - which enabled a diagnostic, originally of type ORIG_DIAG_KIND but - possibly converted to DIAG_KIND by options such as -Werror. + /* Return the name of the option OPT_ID which enabled a diagnostic, + originally of type ORIG_DIAG_KIND but possibly converted to DIAG_KIND + by options such as -Werror. May return NULL if no name is to be printed. May be passed 0 as well as the index of a particular option. */ - virtual char *make_option_name (option_id opt_id, - enum kind orig_diag_kind, - enum kind diag_kind) const = 0; + virtual label_text get_option_name (option_id opt_id, + enum kind orig_diag_kind, + enum kind diag_kind) const = 0; - /* Return malloced memory for a URL describing the option that controls - a diagnostic. + /* Return a URL describing the option that controls a diagnostic. May return NULL if no URL is available. May be passed 0 as well as the index of a particular option. */ - virtual char *make_option_url (option_id opt_id) const = 0; + virtual label_text get_option_url (option_id opt_id) const = 0; }; } // namespace diagnostics diff --git a/gcc/diagnostics/sarif-sink.cc b/gcc/diagnostics/sarif-sink.cc index 1e25b459c64..f601dbe7c5f 100644 --- a/gcc/diagnostics/sarif-sink.cc +++ b/gcc/diagnostics/sarif-sink.cc @@ -37,6 +37,7 @@ struct sockaddr_un { #define INCLUDE_LIST #define INCLUDE_MAP +#define INCLUDE_SET #define INCLUDE_STRING #define INCLUDE_VECTOR #include "system.h" @@ -1012,7 +1013,7 @@ private: sarif_artifact *> m_filename_to_artifact_map; bool m_seen_any_relative_paths; - hash_set m_rule_id_set; + std::set m_rule_id_set; std::unique_ptr m_rules_arr; /* The set of all CWE IDs we've seen, if any. */ @@ -2067,25 +2068,24 @@ sarif_builder::make_result_object (const diagnostic_info &diagnostic, /* "ruleId" property (SARIF v2.1.0 section 3.27.5). */ /* Ideally we'd have an option_name for these. */ - if (char *option_text - = m_context.make_option_name (diagnostic.m_option_id, - orig_diag_kind, diagnostic.m_kind)) + label_text option_text + = m_context.get_option_name (diagnostic.m_option_id, + orig_diag_kind, diagnostic.m_kind); + if (option_text.get ()) { /* Lazily create reportingDescriptor objects for and add to m_rules_arr. Set ruleId referencing them. */ - result_obj->set_string ("ruleId", option_text); - if (m_rule_id_set.contains (option_text)) - free (option_text); - else + result_obj->set_string ("ruleId", option_text.get ()); + if (m_rule_id_set.find (option_text.get ()) == m_rule_id_set.end ()) { /* This is the first time we've seen this ruleId. */ /* Add to set, taking ownership. */ - m_rule_id_set.add (option_text); + m_rule_id_set.insert (option_text.get ()); m_rules_arr->append (make_reporting_descriptor_object_for_warning (diagnostic, orig_diag_kind, - option_text)); + option_text.get ())); } } else @@ -2191,11 +2191,9 @@ make_reporting_descriptor_object_for_warning (const diagnostic_info &diagnostic, it seems redundant compared to "id". */ /* "helpUri" property (SARIF v2.1.0 section 3.49.12). */ - if (char *option_url = m_context.make_option_url (diagnostic.m_option_id)) - { - reporting_desc->set_string ("helpUri", option_url); - free (option_url); - } + label_text option_url = m_context.get_option_url (diagnostic.m_option_id); + if (option_url.get ()) + reporting_desc->set_string ("helpUri", option_url.get ()); return reporting_desc; } diff --git a/gcc/diagnostics/text-sink.cc b/gcc/diagnostics/text-sink.cc index d1bdd15bee3..6f97ca54329 100644 --- a/gcc/diagnostics/text-sink.cc +++ b/gcc/diagnostics/text-sink.cc @@ -552,28 +552,25 @@ void text_sink::print_option_information (const diagnostic_info &diagnostic, enum kind orig_diag_kind) { - if (char *option_text - = m_context.make_option_name (diagnostic.m_option_id, - orig_diag_kind, diagnostic.m_kind)) + label_text option_text + = m_context.get_option_name (diagnostic.m_option_id, + orig_diag_kind, diagnostic.m_kind); + if (option_text.get ()) { - char *option_url = nullptr; + label_text option_url; pretty_printer * const pp = get_printer (); if (pp->supports_urls_p ()) - option_url = m_context.make_option_url (diagnostic.m_option_id); + option_url = m_context.get_option_url (diagnostic.m_option_id); pp_string (pp, " ["); const char *kind_color = get_color_for_kind (diagnostic.m_kind); pp_string (pp, colorize_start (pp_show_color (pp), kind_color)); - if (option_url) - pp_begin_url (pp, option_url); - pp_string (pp, option_text); - if (option_url) - { - pp_end_url (pp); - free (option_url); - } + if (option_url.get ()) + pp_begin_url (pp, option_url.get ()); + pp_string (pp, option_text.get ()); + if (option_url.get ()) + pp_end_url (pp); pp_string (pp, colorize_stop (pp_show_color (pp))); pp_character (pp, ']'); - free (option_text); } } diff --git a/gcc/opts-diagnostic.h b/gcc/opts-diagnostic.h index d0a8beddd81..6c2b25d6342 100644 --- a/gcc/opts-diagnostic.h +++ b/gcc/opts-diagnostic.h @@ -27,7 +27,8 @@ along with GCC; see the file COPYING3. If not see class gcc_diagnostic_option_id_manager : public diagnostics::option_id_manager { public: - char *make_option_url (diagnostics::option_id option_id) const final override; + label_text + get_option_url (diagnostics::option_id option_id) const final override; protected: gcc_diagnostic_option_id_manager (unsigned lang_mask) @@ -53,10 +54,11 @@ public: } int option_enabled_p (diagnostics::option_id option_id) const final override; - char * - make_option_name (diagnostics::option_id option_id, - enum diagnostics::kind orig_diag_kind, - enum diagnostics::kind diag_kind) const final override; + + label_text + get_option_name (diagnostics::option_id option_id, + enum diagnostics::kind orig_diag_kind, + enum diagnostics::kind diag_kind) const final override; private: const diagnostics::context &m_context; diff --git a/gcc/opts.cc b/gcc/opts.cc index 6658b6acd37..09baa411cfd 100644 --- a/gcc/opts.cc +++ b/gcc/opts.cc @@ -3812,16 +3812,15 @@ enable_warning_as_error (const char *arg, int value, unsigned int lang_mask, free (new_option); } -/* Return malloced memory for the name of the option OPTION_INDEX - which enabled a diagnostic, originally of type - ORIG_DIAG_KIND but possibly converted to DIAG_KIND by options such - as -Werror. */ +/* Return the name of the option OPTION_INDEX which enabled a diagnostic, + originally of type ORIG_DIAG_KIND but possibly converted to DIAG_KIND by + options such as -Werror. Can return null if OPTION_ID is zero. */ -char * +label_text compiler_diagnostic_option_id_manager:: -make_option_name (diagnostics::option_id option_id, - enum diagnostics::kind orig_diag_kind, - enum diagnostics::kind diag_kind) const +get_option_name (diagnostics::option_id option_id, + enum diagnostics::kind orig_diag_kind, + enum diagnostics::kind diag_kind) const { if (option_id.m_idx) { @@ -3829,22 +3828,24 @@ make_option_name (diagnostics::option_id option_id, if ((orig_diag_kind == diagnostics::kind::warning || orig_diag_kind == diagnostics::kind::pedwarn) && diag_kind == diagnostics::kind::error) - return concat (cl_options[OPT_Werror_].opt_text, - /* Skip over "-W". */ - cl_options[option_id.m_idx].opt_text + 2, - NULL); + return label_text::take + (concat (cl_options[OPT_Werror_].opt_text, + /* Skip over "-W". */ + cl_options[option_id.m_idx].opt_text + 2, + NULL)); /* A warning with option. */ else - return xstrdup (cl_options[option_id.m_idx].opt_text); + return label_text::take + (xstrdup (cl_options[option_id.m_idx].opt_text)); } /* A warning without option classified as an error. */ else if ((orig_diag_kind == diagnostics::kind::warning || orig_diag_kind == diagnostics::kind::pedwarn || diag_kind == diagnostics::kind::warning) && m_context.warning_as_error_requested_p ()) - return xstrdup (cl_options[OPT_Werror].opt_text); + return label_text::borrow (cl_options[OPT_Werror].opt_text); else - return NULL; + return label_text (); } /* Get the page within the documentation for this option. */ @@ -3893,22 +3894,23 @@ get_option_url_suffix (int option_index, unsigned lang_mask) return label_text (); } -/* Return malloced memory for a URL describing the option OPTION_INDEX - which enabled a diagnostic. */ +/* Return a URL describing the option OPTION_INDEX which enabled + a diagnostic, or null. */ -char * +label_text gcc_diagnostic_option_id_manager:: -make_option_url (diagnostics::option_id option_id) const +get_option_url (diagnostics::option_id option_id) const { if (option_id.m_idx) { label_text url_suffix = get_option_url_suffix (option_id.m_idx, m_lang_mask); if (url_suffix.get ()) - return concat (DOCUMENTATION_ROOT_URL, url_suffix.get (), nullptr); + return label_text::take + (concat (DOCUMENTATION_ROOT_URL, url_suffix.get (), nullptr)); } - return nullptr; + return label_text (); } /* Return a heap allocated producer with command line options. */ -- 2.49.0