* [pushed 1/2] Convert label_text to C++11 move semantics
@ 2022-07-07 20:00 David Malcolm
2022-07-07 20:00 ` [pushed 2/2] analyzer: use label_text for superedge::get_description David Malcolm
2022-07-11 13:48 ` [pushed 1/2] Convert label_text to C++11 move semantics Martin Liška
0 siblings, 2 replies; 3+ messages in thread
From: David Malcolm @ 2022-07-07 20:00 UTC (permalink / raw)
To: gcc-patches
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.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Lightly tested with valgrind.
Pushed to trunk as r13-1563-ga8dce13c076019.
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>
---
gcc/analyzer/call-info.cc | 1 -
gcc/analyzer/checker-path.cc | 97 ++++++++++--------------------
gcc/analyzer/diagnostic-manager.cc | 8 ---
gcc/analyzer/program-state.cc | 1 -
gcc/analyzer/region.cc | 1 -
gcc/analyzer/sm-malloc.cc | 3 -
gcc/analyzer/store.cc | 3 -
gcc/analyzer/svalue.cc | 1 -
gcc/c-family/c-format.cc | 1 -
gcc/diagnostic-format-json.cc | 4 +-
gcc/diagnostic-format-sarif.cc | 1 -
gcc/diagnostic-show-locus.cc | 35 +++++++++--
gcc/tree-diagnostic-path.cc | 4 --
libcpp/include/line-map.h | 46 +++++++++++---
14 files changed, 101 insertions(+), 105 deletions(-)
diff --git a/gcc/analyzer/call-info.cc b/gcc/analyzer/call-info.cc
index b3ff51e7460..e1142d743a3 100644
--- a/gcc/analyzer/call-info.cc
+++ b/gcc/analyzer/call-info.cc
@@ -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
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 953e192cd55..959ffdd853c 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -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 ();
}
}
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 4adfda1af65..083f66bd739 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -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);
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index 295c6aeb185..90a56e3fba4 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -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. */
}
diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc
index a8286231d30..5b00e6a5f46 100644
--- a/gcc/analyzer/region.cc
+++ b/gcc/analyzer/region.cc
@@ -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;
}
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index 3bd40425919..553fcd80085 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -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;
}
diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc
index 1b7c818051b..d558d477115 100644
--- a/gcc/analyzer/store.cc
+++ b/gcc/analyzer/store.cc
@@ -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));
diff --git a/gcc/analyzer/svalue.cc b/gcc/analyzer/svalue.cc
index 7bad3cea31b..78a6eeff05f 100644
--- a/gcc/analyzer/svalue.cc
+++ b/gcc/analyzer/svalue.cc
@@ -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;
}
diff --git a/gcc/c-family/c-format.cc b/gcc/c-family/c-format.cc
index 754780446ba..2faed0c1607 100644
--- a/gcc/c-family/c-format.cc
+++ b/gcc/c-family/c-format.cc
@@ -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);
}
diff --git a/gcc/diagnostic-format-json.cc b/gcc/diagnostic-format-json.cc
index d1d8d3f2081..872c67e53ed 100644
--- a/gcc/diagnostic-format-json.cc
+++ b/gcc/diagnostic-format-json.cc
@@ -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;
diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc
index a7bb9fb639d..1e4ebc8ad38 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -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;
}
diff --git a/gcc/diagnostic-show-locus.cc b/gcc/diagnostic-show-locus.cc
index 6eafe19785f..9cd7794d077 100644
--- a/gcc/diagnostic-show-locus.cc
+++ b/gcc/diagnostic-show-locus.cc
@@ -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)));
}
}
diff --git a/gcc/tree-diagnostic-path.cc b/gcc/tree-diagnostic-path.cc
index ae2f8a2d262..2f297faed34 100644
--- a/gcc/tree-diagnostic-path.cc
+++ b/gcc/tree-diagnostic-path.cc
@@ -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
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 80335721e03..c6379ce25b8 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -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)
{}
};
--
2.26.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pushed 2/2] analyzer: use label_text for superedge::get_description
2022-07-07 20:00 [pushed 1/2] Convert label_text to C++11 move semantics David Malcolm
@ 2022-07-07 20:00 ` David Malcolm
2022-07-11 13:48 ` [pushed 1/2] Convert label_text to C++11 move semantics Martin Liška
1 sibling, 0 replies; 3+ messages in thread
From: David Malcolm @ 2022-07-07 20:00 UTC (permalink / raw)
To: gcc-patches
No functional change intended.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Lightly tested with valgrind.
Pushed to trunk as r13-1564-g52f538fa4a13d5.
gcc/analyzer/ChangeLog:
* checker-path.cc (start_cfg_edge_event::get_desc): Update for
superedge::get_description returning a label_text.
* engine.cc (feasibility_state::maybe_update_for_edge): Likewise.
* supergraph.cc (superedge::dump): Likewise.
(superedge::get_description): Convert return type from char * to
label_text.
* supergraph.h (superedge::get_description): Likewise.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
gcc/analyzer/checker-path.cc | 3 +--
gcc/analyzer/engine.cc | 5 ++---
gcc/analyzer/supergraph.cc | 13 +++++--------
gcc/analyzer/supergraph.h | 2 +-
4 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 959ffdd853c..211cf3e0333 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -594,8 +594,7 @@ label_text
start_cfg_edge_event::get_desc (bool can_colorize) const
{
bool user_facing = !flag_analyzer_verbose_edges;
- label_text edge_desc
- = label_text::take (m_sedge->get_description (user_facing));
+ label_text edge_desc (m_sedge->get_description (user_facing));
if (user_facing)
{
if (edge_desc.m_buffer && strlen (edge_desc.m_buffer) > 0)
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 0674c8ba3b6..888123f2b95 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -4586,12 +4586,11 @@ feasibility_state::maybe_update_for_edge (logger *logger,
{
if (logger)
{
- char *desc = sedge->get_description (false);
+ label_text desc (sedge->get_description (false));
logger->log (" sedge: SN:%i -> SN:%i %s",
sedge->m_src->m_index,
sedge->m_dest->m_index,
- desc);
- free (desc);
+ desc.m_buffer);
}
const gimple *last_stmt = src_point.get_supernode ()->get_last_stmt ();
diff --git a/gcc/analyzer/supergraph.cc b/gcc/analyzer/supergraph.cc
index f023c533a09..52b4852404d 100644
--- a/gcc/analyzer/supergraph.cc
+++ b/gcc/analyzer/supergraph.cc
@@ -854,13 +854,12 @@ void
superedge::dump (pretty_printer *pp) const
{
pp_printf (pp, "edge: SN: %i -> SN: %i", m_src->m_index, m_dest->m_index);
- char *desc = get_description (false);
- if (strlen (desc) > 0)
+ label_text desc (get_description (false));
+ if (strlen (desc.m_buffer) > 0)
{
pp_space (pp);
- pp_string (pp, desc);
+ pp_string (pp, desc.m_buffer);
}
- free (desc);
}
/* Dump this superedge to stderr. */
@@ -998,17 +997,15 @@ superedge::get_any_callgraph_edge () const
/* Build a description of this superedge (e.g. "true" for the true
edge of a conditional, or "case 42:" for a switch case).
- The caller is responsible for freeing the result.
-
If USER_FACING is false, the result also contains any underlying
CFG edge flags. e.g. " (flags FALLTHRU | DFS_BACK)". */
-char *
+label_text
superedge::get_description (bool user_facing) const
{
pretty_printer pp;
dump_label_to_pp (&pp, user_facing);
- return xstrdup (pp_formatted_text (&pp));
+ return label_text::take (xstrdup (pp_formatted_text (&pp)));
}
/* Implementation of superedge::dump_label_to_pp for non-switch CFG
diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h
index 42c6df57435..e9a5be27d88 100644
--- a/gcc/analyzer/supergraph.h
+++ b/gcc/analyzer/supergraph.h
@@ -331,7 +331,7 @@ class superedge : public dedge<supergraph_traits>
::edge get_any_cfg_edge () const;
cgraph_edge *get_any_callgraph_edge () const;
- char *get_description (bool user_facing) const;
+ label_text get_description (bool user_facing) const;
protected:
superedge (supernode *src, supernode *dest, enum edge_kind kind)
--
2.26.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pushed 1/2] Convert label_text to C++11 move semantics
2022-07-07 20:00 [pushed 1/2] Convert label_text to C++11 move semantics David Malcolm
2022-07-07 20:00 ` [pushed 2/2] analyzer: use label_text for superedge::get_description David Malcolm
@ 2022-07-11 13:48 ` Martin Liška
1 sibling, 0 replies; 3+ messages in thread
From: Martin Liška @ 2022-07-11 13:48 UTC (permalink / raw)
To: David Malcolm, gcc-patches
On 7/7/22 22:00, David Malcolm via Gcc-patches wrote:
> |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.|
Hi.
I've just noticed Clang complains about the change:
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-clang/build/libcpp/include/line-map.h:1876:12: warning: moving a temporary object prevents copy elision [-Wpessimizing-move]
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-clang/build/libcpp/include/line-map.h:1882:12: warning: moving a temporary object prevents copy elision [-Wpessimizing-move]
Cheers,
Martin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-11 13:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07 20:00 [pushed 1/2] Convert label_text to C++11 move semantics David Malcolm
2022-07-07 20:00 ` [pushed 2/2] analyzer: use label_text for superedge::get_description David Malcolm
2022-07-11 13:48 ` [pushed 1/2] Convert label_text to C++11 move semantics Martin Liška
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).