On 6/23/21 1:43 AM, Richard Biener wrote:
On Wed, Jun 23, 2021 at 7:23 AM Trevor Saunders <tbsau...@tbsaunde.org> wrote:
On Tue, Jun 22, 2021 at 02:01:24PM -0600, Martin Sebor wrote:
On 6/21/21 1:15 AM, Richard Biener wrote:
[...]
But maybe I'm misunderstanding C++ too much :/
Well, I guess b) from above means auto_vec<> passing to
vec<> taking functions will need changes?
Converting an auto_vec object to a vec slices off its data members.
The auto_vec<T, 0> specialization has no data members so that's not
a bug in and of itself, but auto_vec<T, N> does have data members
so that would be a bug. The risk is not just passing it to
functions by value but also returning it. That risk was made
worse by the addition of the move ctor.
I would agree that the conversion from auto_vec<> to vec<> is
questionable, and should get some work at some point, perhaps just
passingauto_vec references is good enough, or perhaps there is value in
some const_vec view to avoid having to rely on optimizations, I'm not
sure without looking more at the usage.
We do need to be able to provide APIs that work with both auto_vec<>
and vec<>, I agree that those currently taking a vec<> by value are
fragile (and we've had bugs there before), but I'm not ready to say
that changing them all to [const] vec<>& is OK. The alternative
would be passing a const_vec<> by value, passing that along to
const vec<>& APIs should be valid then (I can see quite some API
boundary cleanups being necessary here ...).
But with all this I don't know how to adjust auto_vec<> to no
longer "decay" to vec<> but still being able to pass it to vec<>&
and being able to call vec<> member functions w/o jumping through
hoops. Any hints on that? private inheritance achieves the first
but also hides all the API ...
The simplest way to do that is by preventing the implicit conversion
between the two types and adding a to_vec() member function to
auto_vec as Jason suggested. This exposes the implicit conversions
to the base vec, forcing us to review each and "fix" it either by
changing the argument to either vec& or const vec&, or less often
to avoid the indirection if necessary, by converting the auto_vec
to vec explicitly by calling to_vec(). The attached diff shows
a very rough POC of what that might look like. A side benefit
is that it improves const-safety and makes the effects of functions
on their callers easier to understand.
But that's orthogonal to making auto_vec copy constructible and copy
assignable. That change can be made independently and with much less
effort and no risk.
Martin
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index a671a3eb740..ab6db3860f5 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -759,8 +759,9 @@ extern tree c_finish_omp_clauses (tree, enum c_omp_region_type);
extern tree c_build_va_arg (location_t, tree, location_t, tree);
extern tree c_finish_transaction (location_t, tree, int);
extern bool c_tree_equal (tree, tree);
-extern tree c_build_function_call_vec (location_t, vec<location_t>, tree,
- vec<tree, va_gc> *, vec<tree, va_gc> *);
+extern tree c_build_function_call_vec (location_t, const vec<location_t>&,
+ tree, vec<tree, va_gc> *,
+ vec<tree, va_gc> *);
extern tree c_omp_clause_copy_ctor (tree, tree, tree);
/* Set to 0 at beginning of a function definition, set to 1 if
diff --git a/gcc/dominance.c b/gcc/dominance.c
index 6a262ce8283..cc63391a39a 100644
--- a/gcc/dominance.c
+++ b/gcc/dominance.c
@@ -1227,7 +1227,7 @@ recompute_dominator (enum cdi_direction dir, basic_block bb)
from BBS. */
static void
-prune_bbs_to_update_dominators (vec<basic_block> bbs,
+prune_bbs_to_update_dominators (vec<basic_block> &bbs,
bool conservative)
{
unsigned i;
@@ -1379,7 +1379,7 @@ determine_dominators_for_sons (struct graph *g, vec<basic_block> bbs,
a block of BBS in the current dominance tree dominate it. */
void
-iterate_fix_dominators (enum cdi_direction dir, vec<basic_block> bbs,
+iterate_fix_dominators (enum cdi_direction dir, vec<basic_block> &bbs,
bool conservative)
{
unsigned i;
diff --git a/gcc/dominance.h b/gcc/dominance.h
index 1a8c248ee98..970da02c594 100644
--- a/gcc/dominance.h
+++ b/gcc/dominance.h
@@ -78,7 +78,7 @@ checking_verify_dominators (cdi_direction dir)
basic_block recompute_dominator (enum cdi_direction, basic_block);
extern void iterate_fix_dominators (enum cdi_direction,
- vec<basic_block> , bool);
+ vec<basic_block> &, bool);
extern void add_to_dominance_info (enum cdi_direction, basic_block);
extern void delete_from_dominance_info (enum cdi_direction, basic_block);
extern basic_block first_dom_son (enum cdi_direction, basic_block);
diff --git a/gcc/genautomata.c b/gcc/genautomata.c
index 6bbfc684afa..e488c5f28ef 100644
--- a/gcc/genautomata.c
+++ b/gcc/genautomata.c
@@ -6137,7 +6137,7 @@ evaluate_equiv_classes (automaton_t automaton, vec<state_t> *equiv_classes)
/* The function merges equivalent states of AUTOMATON. */
static void
-merge_states (automaton_t automaton, vec<state_t> equiv_classes)
+merge_states (automaton_t automaton, const vec<state_t> &equiv_classes)
{
state_t curr_state;
state_t new_state;
diff --git a/gcc/genextract.c b/gcc/genextract.c
index 6fe4a2524fc..3ed2f6846c9 100644
--- a/gcc/genextract.c
+++ b/gcc/genextract.c
@@ -214,7 +214,7 @@ VEC_safe_set_locstr (md_rtx_info *info, vec<locstr> *vp,
/* Another helper subroutine of walk_rtx: given a vec<char>, convert it
to a NUL-terminated string in malloc memory. */
static char *
-VEC_char_to_string (vec<char> v)
+VEC_char_to_string (const vec<char> &v)
{
size_t n = v.length ();
char *s = XNEWVEC (char, n + 1);
diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c
index 632947950e4..02ce068d9cf 100644
--- a/gcc/gimple-ssa-store-merging.c
+++ b/gcc/gimple-ssa-store-merging.c
@@ -2654,7 +2654,8 @@ gather_bswap_load_refs (vec<tree> *refs, tree val)
go after the = _5 store and thus change behavior. */
static bool
-check_no_overlap (vec<store_immediate_info *> m_store_info, unsigned int i,
+check_no_overlap (const vec<store_immediate_info *> &m_store_info,
+ unsigned int i,
bool all_integer_cst_p, unsigned int first_order,
unsigned int last_order, unsigned HOST_WIDE_INT start,
unsigned HOST_WIDE_INT end, unsigned int first_earlier,
diff --git a/gcc/gimple.c b/gcc/gimple.c
index f1044e9c630..108daeda43b 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -241,7 +241,7 @@ gimple_build_call_1 (tree fn, unsigned nargs)
specified in vector ARGS. */
gcall *
-gimple_build_call_vec (tree fn, vec<tree> args)
+gimple_build_call_vec (tree fn, const vec<tree> &args)
{
unsigned i;
unsigned nargs = args.length ();
@@ -338,7 +338,7 @@ gimple_build_call_internal (enum internal_fn fn, unsigned nargs, ...)
specified in vector ARGS. */
gcall *
-gimple_build_call_internal_vec (enum internal_fn fn, vec<tree> args)
+gimple_build_call_internal_vec (enum internal_fn fn, const vec<tree> &args)
{
unsigned i, nargs;
gcall *call;
@@ -802,7 +802,7 @@ gimple_build_switch_nlabels (unsigned nlabels, tree index, tree default_label)
ARGS is a vector of labels excluding the default. */
gswitch *
-gimple_build_switch (tree index, tree default_label, vec<tree> args)
+gimple_build_switch (tree index, tree default_label, const vec<tree> &args)
{
unsigned i, nlabels = args.length ();
@@ -3049,7 +3049,7 @@ compare_case_labels (const void *p1, const void *p2)
/* Sort the case labels in LABEL_VEC in place in ascending order. */
void
-sort_case_labels (vec<tree> label_vec)
+sort_case_labels (vec<tree> &label_vec)
{
label_vec.qsort (compare_case_labels);
}
@@ -3074,7 +3074,7 @@ sort_case_labels (vec<tree> label_vec)
found or not. */
void
-preprocess_case_label_vec_for_gimple (vec<tree> labels,
+preprocess_case_label_vec_for_gimple (vec<tree> &labels,
tree index_type,
tree *default_casep)
{
diff --git a/gcc/gimple.h b/gcc/gimple.h
index e7dc2a45a13..aabf68eaea0 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1516,11 +1516,11 @@ void gimple_init (gimple *g, enum gimple_code code, unsigned num_ops);
gimple *gimple_alloc (enum gimple_code, unsigned CXX_MEM_STAT_INFO);
greturn *gimple_build_return (tree);
void gimple_call_reset_alias_info (gcall *);
-gcall *gimple_build_call_vec (tree, vec<tree> );
+gcall *gimple_build_call_vec (tree, const vec<tree> &);
gcall *gimple_build_call (tree, unsigned, ...);
gcall *gimple_build_call_valist (tree, unsigned, va_list);
gcall *gimple_build_call_internal (enum internal_fn, unsigned, ...);
-gcall *gimple_build_call_internal_vec (enum internal_fn, vec<tree> );
+gcall *gimple_build_call_internal_vec (enum internal_fn, const vec<tree> &);
gcall *gimple_build_call_from_tree (tree, tree);
gassign *gimple_build_assign (tree, tree CXX_MEM_STAT_INFO);
gassign *gimple_build_assign (tree, enum tree_code,
@@ -1547,7 +1547,7 @@ gtry *gimple_build_try (gimple_seq, gimple_seq,
gimple *gimple_build_wce (gimple_seq);
gresx *gimple_build_resx (int);
gswitch *gimple_build_switch_nlabels (unsigned, tree, tree);
-gswitch *gimple_build_switch (tree, tree, vec<tree> );
+gswitch *gimple_build_switch (tree, tree, const vec<tree> &);
geh_dispatch *gimple_build_eh_dispatch (int);
gdebug *gimple_build_debug_bind (tree, tree, gimple * CXX_MEM_STAT_INFO);
gdebug *gimple_build_debug_source_bind (tree, tree, gimple * CXX_MEM_STAT_INFO);
@@ -1626,8 +1626,8 @@ extern bool nonbarrier_call_p (gimple *);
extern bool infer_nonnull_range (gimple *, tree);
extern bool infer_nonnull_range_by_dereference (gimple *, tree);
extern bool infer_nonnull_range_by_attribute (gimple *, tree);
-extern void sort_case_labels (vec<tree>);
-extern void preprocess_case_label_vec_for_gimple (vec<tree>, tree, tree *);
+extern void sort_case_labels (vec<tree> &);
+extern void preprocess_case_label_vec_for_gimple (vec<tree> &, tree, tree *);
extern void gimple_seq_set_location (gimple_seq, location_t);
extern void gimple_seq_discard (gimple_seq);
extern void maybe_remove_unused_call_args (struct function *, gimple *);
diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index 9c88765d1fb..a166b706b8a 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -891,7 +891,7 @@ static void move_block_after_check (rtx_insn *);
static void move_succs (vec<edge, va_gc> **, basic_block);
static void sched_remove_insn (rtx_insn *);
static void clear_priorities (rtx_insn *, rtx_vec_t *);
-static void calc_priorities (rtx_vec_t);
+static void calc_priorities (const rtx_vec_t &);
static void add_jump_dependencies (rtx_insn *, rtx_insn *);
#endif /* INSN_SCHEDULING */
@@ -7375,10 +7375,10 @@ haifa_sched_init (void)
basic_block bb;
FOR_EACH_BB_FN (bb, cfun)
bbs.quick_push (bb);
- sched_init_luids (bbs);
+ sched_init_luids (bbs.to_vec ());
sched_deps_init (true);
sched_extend_target ();
- haifa_init_h_i_d (bbs);
+ haifa_init_h_i_d (bbs.to_vec ());
}
sched_init_only_bb = haifa_init_only_bb;
@@ -8923,7 +8923,7 @@ clear_priorities (rtx_insn *insn, rtx_vec_t *roots_ptr)
changed. ROOTS is a vector of instructions whose priority computation will
trigger initialization of all cleared priorities. */
static void
-calc_priorities (rtx_vec_t roots)
+calc_priorities (const rtx_vec_t &roots)
{
int i;
rtx_insn *insn;
@@ -8988,7 +8988,7 @@ sched_init_insn_luid (rtx_insn *insn)
The hook common_sched_info->luid_for_non_insn () is used to determine
if notes, labels, etc. need luids. */
void
-sched_init_luids (bb_vec_t bbs)
+sched_init_luids (const bb_vec_t &bbs)
{
int i;
basic_block bb;
@@ -9062,7 +9062,7 @@ init_h_i_d (rtx_insn *insn)
/* Initialize haifa_insn_data for BBS. */
void
-haifa_init_h_i_d (bb_vec_t bbs)
+haifa_init_h_i_d (const bb_vec_t &bbs)
{
int i;
basic_block bb;
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 3d28a6e8640..511ec564846 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -499,10 +499,10 @@ public:
get reallocated, the member vectors and the underlying auto_vecs would get
out of sync. */
ipa_call_arg_values (ipa_auto_call_arg_values *aavals)
- : m_known_vals (aavals->m_known_vals),
- m_known_contexts (aavals->m_known_contexts),
- m_known_aggs (aavals->m_known_aggs),
- m_known_value_ranges (aavals->m_known_value_ranges)
+ : m_known_vals (aavals->m_known_vals.to_vec ()),
+ m_known_contexts (aavals->m_known_contexts.to_vec ()),
+ m_known_aggs (aavals->m_known_aggs.to_vec ()),
+ m_known_value_ranges (aavals->m_known_value_ranges.to_vec ())
{}
/* If m_known_vals (vector of known "scalar" values) is sufficiantly long,
diff --git a/gcc/ira-build.c b/gcc/ira-build.c
index 4031ce18287..42120656366 100644
--- a/gcc/ira-build.c
+++ b/gcc/ira-build.c
@@ -1672,7 +1672,7 @@ finish_cost_vectors (void)
static vec<ira_loop_tree_node_t>
ira_loop_tree_body_rev_postorder (ira_loop_tree_node_t loop_node ATTRIBUTE_UNUSED,
- vec<ira_loop_tree_node_t> loop_preorder)
+ const vec<ira_loop_tree_node_t> &loop_preorder)
{
vec<ira_loop_tree_node_t> topsort_nodes = vNULL;
unsigned int n_loop_preorder;
diff --git a/gcc/read-rtl.c b/gcc/read-rtl.c
index 925402877ec..e9eb1049e37 100644
--- a/gcc/read-rtl.c
+++ b/gcc/read-rtl.c
@@ -937,7 +937,7 @@ apply_iterators (rtx original, vec<rtx> *queue)
}
if (oname)
- add_overload_instance (oname, iterators, x);
+ add_overload_instance (oname, iterators.to_vec (), x);
/* Add the new rtx to the end of the queue. */
queue->safe_push (x);
diff --git a/gcc/vec.h b/gcc/vec.h
index 30ef9a69473..6c58395ee69 100644
--- a/gcc/vec.h
+++ b/gcc/vec.h
@@ -547,11 +547,7 @@ vec_copy_construct (T *dst, const T *src, unsigned n)
a vec instance, you can assign it the value vNULL. This isn't
needed for file-scope and function-local static vectors, which
are zero-initialized by default. */
-struct vnull
-{
- template <typename T, typename A, typename L>
- CONSTEXPR operator vec<T, A, L> () const { return vec<T, A, L>(); }
-};
+struct vnull { };
extern vnull vNULL;
@@ -1431,10 +1427,25 @@ gt_pch_nx (vec<T, A, vl_embed> *v, gt_pointer_operator op, void *cookie)
As long as we use C++03, we cannot have constructors nor
destructors in classes that are stored in unions. */
+template<typename T, size_t N = 0>
+class auto_vec;
+
template<typename T>
struct vec<T, va_heap, vl_ptr>
{
public:
+ vec () = default;
+ vec (const vec &) = default;
+ vec (vnull): m_vec () { }
+
+ /* Prevent implicit conversion from auto_vec. Use auto_vec::to_vec()
+ instead. */
+ template <size_t N>
+ vec (auto_vec<T, N> &) = delete;
+
+ template <size_t N>
+ void operator= (auto_vec<T, N> &) = delete;
+
/* Memory allocation and deallocation for the embedded vector.
Needed because we cannot have proper ctors/dtors defined. */
void create (unsigned nelems CXX_MEM_STAT_INFO);
@@ -1522,7 +1533,7 @@ public:
want to ask for internal storage for vectors on the stack because if the
size of the vector is larger than the internal storage that space is wasted.
*/
-template<typename T, size_t N = 0>
+template<typename T, size_t N /* = 0 */>
class auto_vec : public vec<T, va_heap>
{
public:
@@ -1549,6 +1560,13 @@ public:
this->release ();
}
+ /* Explicitly convert to the base class. There is no conversion
+ from a const auto_vec because a copy of the returned vec can
+ be used to modify *THIS. */
+ vec<T, va_heap> to_vec () {
+ return *static_cast<vec<T, va_heap> *>(this);
+ }
+
private:
vec<T, va_heap, vl_embed> m_auto;
T m_data[MAX (N - 1, 1)];
@@ -1602,6 +1620,13 @@ public:
return *this;
}
+ /* Explicitly convert to the base class. There is no conversion
+ from a const auto_vec because a copy of the returned vec can
+ be used to modify *THIS. */
+ vec<T, va_heap> to_vec () {
+ return *static_cast<vec<T, va_heap> *>(this);
+ }
+
// You probably don't want to copy a vector, so these are deleted to prevent
// unintentional use. If you really need a copy of the vectors contents you
// can use copy ().
