]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
Convert label_text to C++11 move semantics
authorDavid Malcolm <dmalcolm@redhat.com>
Thu, 7 Jul 2022 19:50:26 +0000 (15:50 -0400)
committerDavid Malcolm <dmalcolm@redhat.com>
Thu, 7 Jul 2022 19:50:26 +0000 (15:50 -0400)
libcpp's class label_text stores a char * for a string and a flag saying
whether it owns the buffer.  I added this class before we could use
C++11, and so to avoid lots of copying it required an explicit call
to label_text::maybe_free to potentially free the buffer.

Now that we can use C++11, this patch removes label_text::maybe_free in
favor of doing the cleanup in the destructor, and using C++ move
semantics to avoid any copying.  This allows lots of messy cleanup code
to be eliminated in favor of implicit destruction (mostly in the
analyzer).

No functional change intended.

gcc/analyzer/ChangeLog:
* call-info.cc (call_info::print): Update for removal of
label_text::maybe_free in favor of automatic memory management.
* checker-path.cc (checker_event::dump): Likewise.
(checker_event::prepare_for_emission): Likewise.
(state_change_event::get_desc): Likewise.
(superedge_event::should_filter_p): Likewise.
(start_cfg_edge_event::get_desc): Likewise.
(warning_event::get_desc): Likewise.
(checker_path::dump): Likewise.
(checker_path::debug): Likewise.
* diagnostic-manager.cc
(diagnostic_manager::prune_for_sm_diagnostic): Likewise.
(diagnostic_manager::prune_interproc_events): Likewise.
* program-state.cc (sm_state_map::to_json): Likewise.
* region.cc (region::to_json): Likewise.
* sm-malloc.cc (inform_nonnull_attribute): Likewise.
* store.cc (binding_map::to_json): Likewise.
(store::to_json): Likewise.
* svalue.cc (svalue::to_json): Likewise.

gcc/c-family/ChangeLog:
* c-format.cc (range_label_for_format_type_mismatch::get_text):
Update for removal of label_text::maybe_free in favor of automatic
memory management.

gcc/ChangeLog:
* diagnostic-format-json.cc (json_from_location_range): Update for
removal of label_text::maybe_free in favor of automatic memory
management.
* diagnostic-format-sarif.cc
(sarif_builder::make_location_object): Likewise.
* diagnostic-show-locus.cc (struct pod_label_text): New.
(class line_label): Convert m_text from label_text to pod_label_text.
(layout::print_any_labels): Move "text" to the line_label.
* tree-diagnostic-path.cc (path_label::get_text): Update for
removal of label_text::maybe_free in favor of automatic memory
management.
(event_range::print): Likewise.
(default_tree_diagnostic_path_printer): Likewise.
(default_tree_make_json_for_path): Likewise.

libcpp/ChangeLog:
* include/line-map.h: Include <utility>.
(class label_text): Delete maybe_free method in favor of a
destructor.  Add move ctor and assignment operator.  Add deletion
of the copy ctor and copy-assignment operator.  Rename field
m_caller_owned to m_owned.  Add std::move where necessary; add
moved_from member function.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
14 files changed:
gcc/analyzer/call-info.cc
gcc/analyzer/checker-path.cc
gcc/analyzer/diagnostic-manager.cc
gcc/analyzer/program-state.cc
gcc/analyzer/region.cc
gcc/analyzer/sm-malloc.cc
gcc/analyzer/store.cc
gcc/analyzer/svalue.cc
gcc/c-family/c-format.cc
gcc/diagnostic-format-json.cc
gcc/diagnostic-format-sarif.cc
gcc/diagnostic-show-locus.cc
gcc/tree-diagnostic-path.cc
libcpp/include/line-map.h

index b3ff51e7460809d3be1e84e3720ad2c4db050e55..e1142d743a3d1db427d52fae888c735796a871cf 100644 (file)
@@ -76,7 +76,6 @@ call_info::print (pretty_printer *pp) const
 {
   label_text desc (get_desc (pp_show_color (pp)));
   pp_string (pp, desc.m_buffer);
-  desc.maybe_free ();
 }
 
 /* Implementation of custom_edge_info::add_events_to_path vfunc for
index 953e192cd55c6728922d2932025a23a466f58ce2..959ffdd853cc98ed74ef39fdc2ccab2362fe72e0 100644 (file)
@@ -196,7 +196,6 @@ checker_event::dump (pretty_printer *pp) const
   label_text event_desc (get_desc (false));
   pp_printf (pp, "\"%s\" (depth %i",
             event_desc.m_buffer, m_effective_depth);
-  event_desc.maybe_free ();
 
   if (m_effective_depth != m_original_depth)
     pp_printf (pp, " corrected from %i",
@@ -235,7 +234,6 @@ checker_event::prepare_for_emission (checker_path *,
   m_emission_id = emission_id;
 
   label_text desc = get_desc (false);
-  desc.maybe_free ();
 }
 
 /* class debug_event : public checker_event.  */
@@ -402,9 +400,8 @@ state_change_event::get_desc (bool can_colorize) const
              meaning.dump_to_pp (&meaning_pp);
 
              /* Append debug version.  */
-             label_text result;
              if (m_origin)
-               result = make_label_text
+               return make_label_text
                  (can_colorize,
                   "%s (state of %qE: %qs -> %qs, origin: %qE, meaning: %s)",
                   custom_desc.m_buffer,
@@ -414,7 +411,7 @@ state_change_event::get_desc (bool can_colorize) const
                   origin,
                   pp_formatted_text (&meaning_pp));
              else
-               result = make_label_text
+               return make_label_text
                  (can_colorize,
                   "%s (state of %qE: %qs -> %qs, NULL origin, meaning: %s)",
                   custom_desc.m_buffer,
@@ -422,9 +419,6 @@ state_change_event::get_desc (bool can_colorize) const
                   m_from->get_name (),
                   m_to->get_name (),
                   pp_formatted_text (&meaning_pp));
-
-             custom_desc.maybe_free ();
-             return result;
            }
          else
            return custom_desc;
@@ -435,28 +429,24 @@ state_change_event::get_desc (bool can_colorize) const
   if (m_sval)
     {
       label_text sval_desc = m_sval->get_desc ();
-      label_text result;
       if (m_origin)
        {
          label_text origin_desc = m_origin->get_desc ();
-         result = make_label_text
+         return make_label_text
            (can_colorize,
             "state of %qs: %qs -> %qs (origin: %qs)",
             sval_desc.m_buffer,
             m_from->get_name (),
             m_to->get_name (),
             origin_desc.m_buffer);
-         origin_desc.maybe_free ();
        }
       else
-       result = make_label_text
+       return make_label_text
          (can_colorize,
           "state of %qs: %qs -> %qs (NULL origin)",
           sval_desc.m_buffer,
           m_from->get_name (),
           m_to->get_name ());
-      sval_desc.maybe_free ();
-      return result;
     }
   else
     {
@@ -522,7 +512,6 @@ superedge_event::should_filter_p (int verbosity) const
            gcc_assert (desc.m_buffer);
            if (desc.m_buffer[0] == '\0')
              return true;
-           desc.maybe_free ();
          }
       }
       break;
@@ -605,56 +594,39 @@ label_text
 start_cfg_edge_event::get_desc (bool can_colorize) const
 {
   bool user_facing = !flag_analyzer_verbose_edges;
-  char *edge_desc = m_sedge->get_description (user_facing);
+  label_text edge_desc
+    = label_text::take (m_sedge->get_description (user_facing));
   if (user_facing)
     {
-      if (edge_desc && strlen (edge_desc) > 0)
+      if (edge_desc.m_buffer && strlen (edge_desc.m_buffer) > 0)
        {
          label_text cond_desc = maybe_describe_condition (can_colorize);
          label_text result;
          if (cond_desc.m_buffer)
-           {
-             result = make_label_text (can_colorize,
-                                       "following %qs branch (%s)...",
-                                       edge_desc, cond_desc.m_buffer);
-             cond_desc.maybe_free ();
-           }
+           return make_label_text (can_colorize,
+                                   "following %qs branch (%s)...",
+                                   edge_desc.m_buffer, cond_desc.m_buffer);
          else
-           {
-             result = make_label_text (can_colorize,
-                                       "following %qs branch...",
-                                       edge_desc);
-           }
-         free (edge_desc);
-         return result;
+           return make_label_text (can_colorize,
+                                   "following %qs branch...",
+                                   edge_desc.m_buffer);
        }
       else
-       {
-         free (edge_desc);
-         return label_text::borrow ("");
-       }
+       return label_text::borrow ("");
     }
   else
     {
-      if (strlen (edge_desc) > 0)
-       {
-         label_text result
-           = make_label_text (can_colorize,
-                              "taking %qs edge SN:%i -> SN:%i",
-                              edge_desc,
-                              m_sedge->m_src->m_index,
-                              m_sedge->m_dest->m_index);
-         free (edge_desc);
-         return result;
-       }
+      if (strlen (edge_desc.m_buffer) > 0)
+       return make_label_text (can_colorize,
+                               "taking %qs edge SN:%i -> SN:%i",
+                               edge_desc.m_buffer,
+                               m_sedge->m_src->m_index,
+                               m_sedge->m_dest->m_index);
       else
-       {
-         free (edge_desc);
-         return make_label_text (can_colorize,
-                                 "taking edge SN:%i -> SN:%i",
-                                 m_sedge->m_src->m_index,
-                                 m_sedge->m_dest->m_index);
-       }
+       return make_label_text (can_colorize,
+                               "taking edge SN:%i -> SN:%i",
+                               m_sedge->m_src->m_index,
+                               m_sedge->m_dest->m_index);
     }
 }
 
@@ -1138,19 +1110,16 @@ warning_event::get_desc (bool can_colorize) const
        {
          if (m_sm && flag_analyzer_verbose_state_changes)
            {
-             label_text result;
              if (var)
-               result = make_label_text (can_colorize,
-                                         "%s (%qE is in state %qs)",
-                                         ev_desc.m_buffer,
-                                         var, m_state->get_name ());
+               return make_label_text (can_colorize,
+                                       "%s (%qE is in state %qs)",
+                                       ev_desc.m_buffer,
+                                       var, m_state->get_name ());
              else
-               result = make_label_text (can_colorize,
-                                         "%s (in global state %qs)",
-                                         ev_desc.m_buffer,
-                                         m_state->get_name ());
-             ev_desc.maybe_free ();
-             return result;
+               return make_label_text (can_colorize,
+                                       "%s (in global state %qs)",
+                                       ev_desc.m_buffer,
+                                       m_state->get_name ());
            }
          else
            return ev_desc;
@@ -1196,7 +1165,6 @@ checker_path::dump (pretty_printer *pp) const
        pp_string (pp, ", ");
       label_text event_desc (e->get_desc (false));
       pp_printf (pp, "\"%s\"", event_desc.m_buffer);
-      event_desc.maybe_free ();
     }
   pp_character (pp, ']');
 }
@@ -1237,7 +1205,6 @@ checker_path::debug () const
               i,
               event_kind_to_string (m_events[i]->m_kind),
               event_desc.m_buffer);
-      event_desc.maybe_free ();
     }
 }
 
index 4adfda1af65721db6ac5931b3c0178d3bfa352cf..083f66bd739a41b60dcbd954167e5982ee4c2e4c 100644 (file)
@@ -2298,7 +2298,6 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
                  log ("considering event %i (%s), with sval: %qs, state: %qs",
                       idx, event_kind_to_string (base_event->m_kind),
                       sval_desc.m_buffer, state->get_name ());
-                 sval_desc.maybe_free ();
                }
              else
                log ("considering event %i (%s), with global state: %qs",
@@ -2366,8 +2365,6 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
                             " switching var of interest from %qs to %qs",
                             idx, sval_desc.m_buffer,
                             origin_sval_desc.m_buffer);
-                       sval_desc.maybe_free ();
-                       origin_sval_desc.maybe_free ();
                      }
                    sval = state_change->m_origin;
                  }
@@ -2395,7 +2392,6 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
                        else
                          log ("filtering event %i: state change to %qs",
                               idx, change_sval_desc.m_buffer);
-                       change_sval_desc.maybe_free ();
                      }
                    else
                      log ("filtering event %i: global state change", idx);
@@ -2465,7 +2461,6 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
                         " recording critical state for %qs at call"
                         " from %qE in callee to %qE in caller",
                         idx, sval_desc.m_buffer, callee_var, caller_var);
-                   sval_desc.maybe_free ();
                  }
                if (expr.param_p ())
                  event->record_critical_state (caller_var, state);
@@ -2509,7 +2504,6 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
                             " recording critical state for %qs at return"
                             " from %qE in caller to %qE in callee",
                             idx, sval_desc.m_buffer, callee_var, callee_var);
-                       sval_desc.maybe_free ();
                      }
                    if (expr.return_value_p ())
                      event->record_critical_state (callee_var, state);
@@ -2593,7 +2587,6 @@ diagnostic_manager::prune_interproc_events (checker_path *path) const
                  log ("filtering events %i-%i:"
                       " irrelevant call/entry/return: %s",
                       idx, idx + 2, desc.m_buffer);
-                 desc.maybe_free ();
                }
              path->delete_event (idx + 2);
              path->delete_event (idx + 1);
@@ -2616,7 +2609,6 @@ diagnostic_manager::prune_interproc_events (checker_path *path) const
                  log ("filtering events %i-%i:"
                       " irrelevant call/return: %s",
                       idx, idx + 1, desc.m_buffer);
-                 desc.maybe_free ();
                }
              path->delete_event (idx + 1);
              path->delete_event (idx);
index 295c6aeb185cb6f4edd0203e9f1e5bb0a7835a97..90a56e3fba4a14e9c56b8874ac99af21e294b93f 100644 (file)
@@ -301,7 +301,6 @@ sm_state_map::to_json () const
 
       label_text sval_desc = sval->get_desc ();
       map_obj->set (sval_desc.m_buffer, e.m_state->to_json ());
-      sval_desc.maybe_free ();
 
       /* This doesn't yet JSONify e.m_origin.  */
     }
index a8286231d30e0098d042fd229620cabe1fa83e29..5b00e6a5f462c1fcd16e432487cd6dfe4fc58b34 100644 (file)
@@ -590,7 +590,6 @@ region::to_json () const
 {
   label_text desc = get_desc (true);
   json::value *reg_js = new json::string (desc.m_buffer);
-  desc.maybe_free ();
   return reg_js;
 }
 
index 3bd40425919a3bc51bec5947c324f23db1b2f3a0..553fcd80085841afe4ce21af90eea623e5b65cd4 100644 (file)
@@ -1008,7 +1008,6 @@ inform_nonnull_attribute (tree fndecl, int arg_idx)
   inform (DECL_SOURCE_LOCATION (fndecl),
          "argument %s of %qD must be non-null",
          arg_desc.m_buffer, fndecl);
-  arg_desc.maybe_free ();
   /* Ideally we would use the location of the parm and underline the
      attribute also - but we don't have the location_t values at this point
      in the middle-end.
@@ -1072,7 +1071,6 @@ public:
       result = ev.formatted_print ("argument %s (%qE) could be NULL"
                                   " where non-null expected",
                                   arg_desc.m_buffer, ev.m_expr);
-    arg_desc.maybe_free ();
     return result;
   }
 
@@ -1180,7 +1178,6 @@ public:
       result = ev.formatted_print ("argument %s (%qE) NULL"
                                   " where non-null expected",
                                   arg_desc.m_buffer, ev.m_expr);
-    arg_desc.maybe_free ();
     return result;
   }
 
index 1b7c818051b6b05d4f8d850a65084f7644a70182..d558d477115e059992e074f5ab09c48ca041b5a8 100644 (file)
@@ -676,7 +676,6 @@ binding_map::to_json () const
       const svalue *value = *const_cast <map_t &> (m_map).get (key);
       label_text key_desc = key->get_desc ();
       map_obj->set (key_desc.m_buffer, value->to_json ());
-      key_desc.maybe_free ();
     }
 
   return map_obj;
@@ -2405,11 +2404,9 @@ store::to_json () const
          label_text base_reg_desc = base_reg->get_desc ();
          clusters_in_parent_reg_obj->set (base_reg_desc.m_buffer,
                                           cluster->to_json ());
-         base_reg_desc.maybe_free ();
        }
       label_text parent_reg_desc = parent_reg->get_desc ();
       store_obj->set (parent_reg_desc.m_buffer, clusters_in_parent_reg_obj);
-      parent_reg_desc.maybe_free ();
     }
 
   store_obj->set ("called_unknown_fn", new json::literal (m_called_unknown_fn));
index 7bad3cea31b4c07ecc3716b0c57941f8416df036..78a6eeff05f36db15a255b76a420f793e6012baf 100644 (file)
@@ -97,7 +97,6 @@ svalue::to_json () const
 {
   label_text desc = get_desc (true);
   json::value *sval_js = new json::string (desc.m_buffer);
-  desc.maybe_free ();
   return sval_js;
 }
 
index 754780446bada05bb94deebef126b5f73a3f75f3..2faed0c1607c247967fda80b0af39c6e154dfa7e 100644 (file)
@@ -4625,7 +4625,6 @@ class range_label_for_format_type_mismatch
     suffix.fill_buffer (p);
 
     char *result = concat (text.m_buffer, p, NULL);
-    text.maybe_free ();
     return label_text::take (result);
   }
 
index d1d8d3f2081172e1a24c868045625d9d264fccf5..872c67e53edcd94783999ad337a2e464303b0ab5 100644 (file)
@@ -101,11 +101,9 @@ json_from_location_range (diagnostic_context *context,
 
   if (loc_range->m_label)
     {
-      label_text text;
-      text = loc_range->m_label->get_text (range_idx);
+      label_text text (loc_range->m_label->get_text (range_idx));
       if (text.m_buffer)
        result->set ("label", new json::string (text.m_buffer));
-      text.maybe_free ();
     }
 
   return result;
index a7bb9fb639d6dc37fd7559525f25fe26fc3dac91..1e4ebc8ad38d539bde7d6be49928567749a525f5 100644 (file)
@@ -584,7 +584,6 @@ sarif_builder::make_location_object (const diagnostic_event &event)
   label_text ev_desc = event.get_desc (false);
   json::object *message_obj = make_message_object (ev_desc.m_buffer);
   location_obj->set ("message", message_obj);
-  ev_desc.maybe_free ();
 
   return location_obj;
 }
index 6eafe19785ffc46dd6f5be4787853ba74a403bdc..9cd7794d077811bf053100b79db759022a89a00e 100644 (file)
@@ -1867,6 +1867,31 @@ layout::print_annotation_line (linenum_type row, const line_bounds lbounds)
   print_newline ();
 }
 
+/* A version of label_text that can live inside a vec.
+   Requires manual cleanup via maybe_free.  */
+
+struct pod_label_text
+{
+  pod_label_text ()
+  : m_buffer (NULL), m_caller_owned (false)
+  {}
+
+  pod_label_text (label_text &&other)
+  : m_buffer (other.m_buffer), m_caller_owned (other.m_owned)
+  {
+    other.moved_from ();
+  }
+
+  void maybe_free ()
+  {
+    if (m_caller_owned)
+      free (m_buffer);
+  }
+
+  char *m_buffer;
+  bool m_caller_owned;
+};
+
 /* Implementation detail of layout::print_any_labels.
 
    A label within the given row of source.  */
@@ -1878,10 +1903,10 @@ public:
              int state_idx, int column,
              label_text text)
   : m_state_idx (state_idx), m_column (column),
-    m_text (text), m_label_line (0), m_has_vbar (true)
+    m_text (std::move (text)), m_label_line (0), m_has_vbar (true)
   {
-    const int bytes = strlen (text.m_buffer);
-    m_display_width = cpp_display_width (text.m_buffer, bytes, policy);
+    const int bytes = strlen (m_text.m_buffer);
+    m_display_width = cpp_display_width (m_text.m_buffer, bytes, policy);
   }
 
   /* Sorting is primarily by column, then by state index.  */
@@ -1900,7 +1925,7 @@ public:
 
   int m_state_idx;
   int m_column;
-  label_text m_text;
+  pod_label_text m_text;
   size_t m_display_width;
   int m_label_line;
   bool m_has_vbar;
@@ -1941,7 +1966,7 @@ layout::print_any_labels (linenum_type row)
        if (text.m_buffer == NULL)
          continue;
 
-       labels.safe_push (line_label (m_policy, i, disp_col, text));
+       labels.safe_push (line_label (m_policy, i, disp_col, std::move (text)));
       }
   }
 
index ae2f8a2d26279d33515469ef717301538edd2c8c..2f297faed349b3bf296f09440310ded61d2ac16f 100644 (file)
@@ -66,7 +66,6 @@ class path_label : public range_label
     pp_show_color (&pp) = pp_show_color (global_dc->printer);
     diagnostic_event_id_t event_id (event_idx);
     pp_printf (&pp, "%@ %s", &event_id, event_text.m_buffer);
-    event_text.maybe_free ();
     label_text result = label_text::take (xstrdup (pp_formatted_text (&pp)));
     return result;
   }
@@ -176,7 +175,6 @@ struct event_range
            pretty_printer *pp = dc->printer;
            pp_printf (pp, " %@: %s", &event_id, event_text.m_buffer);
            pp_newline (pp);
-           event_text.maybe_free ();
          }
        return;
       }
@@ -484,7 +482,6 @@ default_tree_diagnostic_path_printer (diagnostic_context *context,
            else
              inform (event.get_location (),
                      "%@ %s", &event_id, event_text.m_buffer);
-           event_text.maybe_free ();
          }
       }
       break;
@@ -523,7 +520,6 @@ default_tree_make_json_for_path (diagnostic_context *context,
                                                     event.get_location ()));
       label_text event_text (event.get_desc (false));
       event_obj->set ("description", new json::string (event_text.m_buffer));
-      event_text.maybe_free ();
       if (tree fndecl = event.get_fndecl ())
        {
          const char *function
index 80335721e037adf62bb77ad0b5046a6a7542f9db..c6379ce25b820883bd757dc0db29ef4a47cc14d1 100644 (file)
@@ -22,6 +22,8 @@ along with this program; see the file COPYING3.  If not see
 #ifndef LIBCPP_LINE_MAP_H
 #define LIBCPP_LINE_MAP_H
 
+#include <utility>
+
 #ifndef GTY
 #define GTY(x) /* nothing */
 #endif
@@ -1836,43 +1838,71 @@ class label_text
 {
 public:
   label_text ()
-  : m_buffer (NULL), m_caller_owned (false)
+  : m_buffer (NULL), m_owned (false)
   {}
 
-  void maybe_free ()
+  ~label_text ()
+  {
+    if (m_owned)
+      free (m_buffer);
+  }
+
+  /* Move ctor.  */
+  label_text (label_text &&other)
+  : m_buffer (other.m_buffer), m_owned (other.m_owned)
+  {
+    other.moved_from ();
+  }
+
+  /* Move assignment.  */
+  label_text & operator= (label_text &&other)
   {
-    if (m_caller_owned)
+    if (m_owned)
       free (m_buffer);
+    m_buffer = other.m_buffer;
+    m_owned = other.m_owned;
+    other.moved_from ();
+    return *this;
   }
 
+  /* Delete the copy ctor and copy-assignment operator.  */
+  label_text (const label_text &) = delete;
+  label_text & operator= (const label_text &) = delete;
+
   /* Create a label_text instance that borrows BUFFER from a
      longer-lived owner.  */
   static label_text borrow (const char *buffer)
   {
-    return label_text (const_cast <char *> (buffer), false);
+    return std::move (label_text (const_cast <char *> (buffer), false));
   }
 
   /* Create a label_text instance that takes ownership of BUFFER.  */
   static label_text take (char *buffer)
   {
-    return label_text (buffer, true);
+    return std::move (label_text (buffer, true));
   }
 
   /* Take ownership of the buffer, copying if necessary.  */
   char *take_or_copy ()
   {
-    if (m_caller_owned)
+    if (m_owned)
       return m_buffer;
     else
       return xstrdup (m_buffer);
   }
 
+  void moved_from ()
+  {
+    m_buffer = NULL;
+    m_owned = false;
+  }
+
   char *m_buffer;
-  bool m_caller_owned;
+  bool m_owned;
 
 private:
   label_text (char *buffer, bool owned)
-  : m_buffer (buffer), m_caller_owned (owned)
+  : m_buffer (buffer), m_owned (owned)
   {}
 };