* [PATCH] compile: Remove non-const reference parameters
@ 2018-08-30 16:47 Simon Marchi
2018-08-30 21:57 ` Keith Seitz
2018-09-17 6:32 ` Tom Tromey
0 siblings, 2 replies; 11+ messages in thread
From: Simon Marchi @ 2018-08-30 16:47 UTC (permalink / raw)
To: gdb-patches; +Cc: Keith Seitz, Simon Marchi
As mentioned here:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Avoid_non-const_reference_parameters.2C_use_pointers_instead
we prefer to avoid non-const references. This patch changes the
non-const references I could find in the compile/ directory, either by
making them const or changing them to pointers.
I'd say all the changes are pretty obvious, except the one in
compile_cplus_instance::enter_scope which might require more attention.
gdb/ChangeLog:
* compile/compile-c.h (generate_c_for_variable_locations):
Change reference to pointer.
* compile/compile-c-support.c (compile_program) <compute>:
Likewise.
* compile/compile-c-symbols.c (generate_vla_size): Likewise.
(generate_c_for_for_one_variable): Likewise
(generate_c_for_variable_locations): Likewise
* compile/compile-c-types.c (compile_c_instance::convert_type):
Likewise
* compile/compile-cplus-types.c
(compile_cplus_instance::enter_scope): Make reference const.
(compile_cplus_instance::new_scope): Change reference to
pointer.
(compile_cplus_instance::convert_type): Likewise
* compile/compile-cplus.h (compile_cplus_instance)
<enter_scope>: Make reference const.
* compile/compile-internal.h (compile_instance)
<get_cached_type>: Likewise
* compile/compile-loc2c.c (push): Likewise
(pushf): Likewise
(unary): Likewise
(binary): Likewise
(print_label): Likewise
(pushf_register_address): Likewise
(pushf_register): Likewise
(do_compile_dwarf_expr_to_c): Likewise
(compile_dwarf_expr_to_c): Likewise
(compile_dwarf_bounds_to_c): Likewise
* compile/compile.c (compile_instance::get_cached_type):
Likewise
* compile/compile.h (compile_dwarf_expr_to_c): Likewise.
(compile_dwarf_bounds_to_c): Likewise
* dwarf2loc.c (locexpr_generate_c_location): Likewise.
(dwarf2_compile_property_to_c): Likewise
* dwarf2loc.h (dwarf2_compile_property_to_c): Likewise
* symtab.h (struct symbol_computed_ops) <generate_c_location>:
Likewise
---
gdb/compile/compile-c-support.c | 2 +-
gdb/compile/compile-c-symbols.c | 14 ++---
gdb/compile/compile-c-types.c | 2 +-
gdb/compile/compile-c.h | 2 +-
gdb/compile/compile-cplus-types.c | 9 ++--
gdb/compile/compile-cplus.h | 2 +-
gdb/compile/compile-internal.h | 2 +-
gdb/compile/compile-loc2c.c | 106 +++++++++++++++++++-------------------
gdb/compile/compile.c | 4 +-
gdb/compile/compile.h | 4 +-
gdb/dwarf2loc.c | 6 +--
gdb/dwarf2loc.h | 2 +-
gdb/symtab.h | 2 +-
13 files changed, 78 insertions(+), 79 deletions(-)
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 5c700bb..28a07dc 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -572,7 +572,7 @@ public:
register struct before the function body. This requires a
temporary stream. */
gdb::unique_xmalloc_ptr<unsigned char> registers_used
- = generate_c_for_variable_locations (m_instance, var_stream, m_arch,
+ = generate_c_for_variable_locations (m_instance, &var_stream, m_arch,
expr_block, expr_pc);
buf.puts ("typedef unsigned int"
diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index e7423d1..e877b78 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -487,7 +487,7 @@ symbol_seen (htab_t hashtab, struct symbol *sym)
static void
generate_vla_size (compile_instance *compiler,
- string_file &stream,
+ string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc,
@@ -541,7 +541,7 @@ generate_vla_size (compile_instance *compiler,
static void
generate_c_for_for_one_variable (compile_instance *compiler,
- string_file &stream,
+ string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc,
@@ -556,10 +556,10 @@ generate_c_for_for_one_variable (compile_instance *compiler,
occurs in the middle. */
string_file local_file;
- generate_vla_size (compiler, local_file, gdbarch, registers_used, pc,
+ generate_vla_size (compiler, &local_file, gdbarch, registers_used, pc,
SYMBOL_TYPE (sym), sym);
- stream.write (local_file.c_str (), local_file.size ());
+ stream->write (local_file.c_str (), local_file.size ());
}
if (SYMBOL_COMPUTED_OPS (sym) != NULL)
@@ -570,12 +570,12 @@ generate_c_for_for_one_variable (compile_instance *compiler,
occurs in the middle. */
string_file local_file;
- SYMBOL_COMPUTED_OPS (sym)->generate_c_location (sym, local_file,
+ SYMBOL_COMPUTED_OPS (sym)->generate_c_location (sym, &local_file,
gdbarch,
registers_used,
pc,
generated_name.get ());
- stream.write (local_file.c_str (), local_file.size ());
+ stream->write (local_file.c_str (), local_file.size ());
}
else
{
@@ -611,7 +611,7 @@ generate_c_for_for_one_variable (compile_instance *compiler,
gdb::unique_xmalloc_ptr<unsigned char>
generate_c_for_variable_locations (compile_instance *compiler,
- string_file &stream,
+ string_file *stream,
struct gdbarch *gdbarch,
const struct block *block,
CORE_ADDR pc)
diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
index 30a4fcb..33b4a87 100644
--- a/gdb/compile/compile-c-types.c
+++ b/gdb/compile/compile-c-types.c
@@ -353,7 +353,7 @@ compile_c_instance::convert_type (struct type *type)
type = check_typedef (type);
gcc_type result;
- if (get_cached_type (type, result))
+ if (get_cached_type (type, &result))
return result;
result = convert_type_basic (this, type);
diff --git a/gdb/compile/compile-c.h b/gdb/compile/compile-c.h
index 18ff4d3..aa11a13 100644
--- a/gdb/compile/compile-c.h
+++ b/gdb/compile/compile-c.h
@@ -69,7 +69,7 @@ private:
extern gdb::unique_xmalloc_ptr<unsigned char>
generate_c_for_variable_locations
(compile_instance *compiler,
- string_file &stream,
+ string_file *stream,
struct gdbarch *gdbarch,
const struct block *block,
CORE_ADDR pc);
diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index 7fc4136..ab4a48b 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -247,14 +247,13 @@ operator!= (const compile_scope &lhs, const compile_scope &rhs)
/* See description in compile-cplus.h. */
void
-compile_cplus_instance::enter_scope (compile_scope &new_scope)
+compile_cplus_instance::enter_scope (const compile_scope &new_scope)
{
bool must_push = m_scopes.empty () || m_scopes.back () != new_scope;
- new_scope.m_pushed = must_push;
-
/* Save the new scope. */
m_scopes.push_back (new_scope);
+ m_scopes.back().m_pushed = must_push;
if (must_push)
{
@@ -360,7 +359,7 @@ compile_cplus_instance::new_scope (const char *type_name, struct type *type)
class, the previous call will give us that type's gcc_type.
Upper layers are expecting to get the original type's
gcc_type! */
- get_cached_type (type, scope.m_nested_type);
+ get_cached_type (type, &scope.m_nested_type);
return scope;
}
}
@@ -1211,7 +1210,7 @@ compile_cplus_instance::convert_type (struct type *type,
{
/* Check if TYPE has already been converted. */
gcc_type result;
- if (get_cached_type (type, result))
+ if (get_cached_type (type, &result))
return result;
/* It is the first time this type has been seen -- convert it
diff --git a/gdb/compile/compile-cplus.h b/gdb/compile/compile-cplus.h
index 7baa57d..b7c4df5 100644
--- a/gdb/compile/compile-cplus.h
+++ b/gdb/compile/compile-cplus.h
@@ -159,7 +159,7 @@ public:
compile_scope new_scope (const char *type_name, struct type *type);
/* Enter the given NEW_SCOPE. */
- void enter_scope (compile_scope &scope);
+ void enter_scope (const compile_scope &scope);
/* Leave the current scope. */
void leave_scope ();
diff --git a/gdb/compile/compile-internal.h b/gdb/compile/compile-internal.h
index a6e7330..2df8b89 100644
--- a/gdb/compile/compile-internal.h
+++ b/gdb/compile/compile-internal.h
@@ -59,7 +59,7 @@ public:
/* Query the type cache for TYPE, returning the compiler's
type for it in RET. */
- bool get_cached_type (struct type *type, gcc_type &ret) const;
+ bool get_cached_type (struct type *type, gcc_type *ret) const;
/* Insert GCC_TYPE into the type cache for TYPE.
diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
index bd080f8..1997e9a 100644
--- a/gdb/compile/compile-loc2c.c
+++ b/gdb/compile/compile-loc2c.c
@@ -430,9 +430,9 @@ compute_stack_depth (enum bfd_endian byte_order, unsigned int addr_size,
/* Emit code to push a constant. */
static void
-push (int indent, string_file &stream, ULONGEST l)
+push (int indent, string_file *stream, ULONGEST l)
{
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_stack[++__gdb_tos] = (" GCC_UINTPTR ") %s;\n",
hex_string (l));
}
@@ -440,57 +440,57 @@ push (int indent, string_file &stream, ULONGEST l)
/* Emit code to push an arbitrary expression. This works like
printf. */
-static void pushf (int indent, string_file &stream, const char *format, ...)
+static void pushf (int indent, string_file *stream, const char *format, ...)
ATTRIBUTE_PRINTF (3, 4);
static void
-pushf (int indent, string_file &stream, const char *format, ...)
+pushf (int indent, string_file *stream, const char *format, ...)
{
va_list args;
- fprintfi_filtered (indent, &stream, "__gdb_stack[__gdb_tos + 1] = ");
+ fprintfi_filtered (indent, stream, "__gdb_stack[__gdb_tos + 1] = ");
va_start (args, format);
- stream.vprintf (format, args);
+ stream->vprintf (format, args);
va_end (args);
- stream.puts (";\n");
+ stream->puts (";\n");
- fprintfi_filtered (indent, &stream, "++__gdb_tos;\n");
+ fprintfi_filtered (indent, stream, "++__gdb_tos;\n");
}
/* Emit code for a unary expression -- one which operates in-place on
the top-of-stack. This works like printf. */
-static void unary (int indent, string_file &stream, const char *format, ...)
+static void unary (int indent, string_file *stream, const char *format, ...)
ATTRIBUTE_PRINTF (3, 4);
static void
-unary (int indent, string_file &stream, const char *format, ...)
+unary (int indent, string_file *stream, const char *format, ...)
{
va_list args;
- fprintfi_filtered (indent, &stream, "__gdb_stack[__gdb_tos] = ");
+ fprintfi_filtered (indent, stream, "__gdb_stack[__gdb_tos] = ");
va_start (args, format);
- stream.vprintf (format, args);
+ stream->vprintf (format, args);
va_end (args);
- stream.puts (";\n");
+ stream->puts (";\n");
}
/* Emit code for a unary expression -- one which uses the top two
stack items, popping the topmost one. This works like printf. */
-static void binary (int indent, string_file &stream, const char *format, ...)
+static void binary (int indent, string_file *stream, const char *format, ...)
ATTRIBUTE_PRINTF (3, 4);
static void
-binary (int indent, string_file &stream, const char *format, ...)
+binary (int indent, string_file *stream, const char *format, ...)
{
va_list args;
- fprintfi_filtered (indent, &stream, "__gdb_stack[__gdb_tos - 1] = ");
+ fprintfi_filtered (indent, stream, "__gdb_stack[__gdb_tos - 1] = ");
va_start (args, format);
- stream.vprintf (format, args);
+ stream->vprintf (format, args);
va_end (args);
- stream.puts (";\n");
- fprintfi_filtered (indent, &stream, "--__gdb_tos;\n");
+ stream->puts (";\n");
+ fprintfi_filtered (indent, stream, "--__gdb_tos;\n");
}
/* Print the name of a label given its "SCOPE", an arbitrary integer
@@ -498,9 +498,9 @@ binary (int indent, string_file &stream, const char *format, ...)
corresponding to the label's point of definition. */
static void
-print_label (string_file &stream, unsigned int scope, int target)
+print_label (string_file *stream, unsigned int scope, int target)
{
- stream.printf ("__label_%u_%s", scope, pulongest (target));
+ stream->printf ("__label_%u_%s", scope, pulongest (target));
}
/* Emit code that pushes a register's address on the stack.
@@ -508,7 +508,7 @@ print_label (string_file &stream, unsigned int scope, int target)
register was needed by this expression. */
static void
-pushf_register_address (int indent, string_file &stream,
+pushf_register_address (int indent, string_file *stream,
unsigned char *registers_used,
struct gdbarch *gdbarch, int regnum)
{
@@ -526,7 +526,7 @@ pushf_register_address (int indent, string_file &stream,
register's value before it is pushed. */
static void
-pushf_register (int indent, string_file &stream,
+pushf_register (int indent, string_file *stream,
unsigned char *registers_used,
struct gdbarch *gdbarch, int regnum, uint64_t offset)
{
@@ -572,7 +572,7 @@ pushf_register (int indent, string_file &stream,
things. */
static void
-do_compile_dwarf_expr_to_c (int indent, string_file &stream,
+do_compile_dwarf_expr_to_c (int indent, string_file *stream,
const char *type_name,
const char *result_name,
struct symbol *sym, CORE_ADDR pc,
@@ -596,9 +596,9 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
++scope;
- fprintfi_filtered (indent, &stream, "__attribute__ ((unused)) %s %s;\n",
+ fprintfi_filtered (indent, stream, "__attribute__ ((unused)) %s %s;\n",
type_name, result_name);
- fprintfi_filtered (indent, &stream, "{\n");
+ fprintfi_filtered (indent, stream, "{\n");
indent += 2;
stack_depth = compute_stack_depth (byte_order, addr_size,
@@ -634,19 +634,19 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
"compiled code."),
SYMBOL_PRINT_NAME (sym));
- fprintfi_filtered (indent, &stream, "%s = %s;\n",
+ fprintfi_filtered (indent, stream, "%s = %s;\n",
result_name,
core_addr_to_string (value_address (val)));
- fprintfi_filtered (indent - 2, &stream, "}\n");
+ fprintfi_filtered (indent - 2, stream, "}\n");
return;
}
- fprintfi_filtered (indent, &stream, GCC_UINTPTR " __gdb_stack[%d];\n",
+ fprintfi_filtered (indent, stream, GCC_UINTPTR " __gdb_stack[%d];\n",
stack_depth);
if (need_tempvar)
- fprintfi_filtered (indent, &stream, GCC_UINTPTR " __gdb_tmp;\n");
- fprintfi_filtered (indent, &stream, "int __gdb_tos = -1;\n");
+ fprintfi_filtered (indent, stream, GCC_UINTPTR " __gdb_tmp;\n");
+ fprintfi_filtered (indent, stream, "int __gdb_tos = -1;\n");
if (initial != NULL)
pushf (indent, stream, "%s", core_addr_to_string (*initial));
@@ -657,13 +657,13 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
uint64_t uoffset, reg;
int64_t offset;
- print_spaces (indent - 2, &stream);
+ print_spaces (indent - 2, stream);
if (info[op_ptr - base].label)
{
print_label (stream, scope, op_ptr - base);
- stream.puts (":;");
+ stream->puts (":;");
}
- stream.printf ("/* %s */\n", get_DW_OP_name (op));
+ stream->printf ("/* %s */\n", get_DW_OP_name (op));
/* This is handy for debugging the generated code:
fprintf_filtered (stream, "if (__gdb_tos != %d) abort ();\n",
@@ -905,7 +905,7 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
break;
case DW_OP_drop:
- fprintfi_filtered (indent, &stream, "--__gdb_tos;\n");
+ fprintfi_filtered (indent, stream, "--__gdb_tos;\n");
break;
case DW_OP_pick:
@@ -915,13 +915,13 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
break;
case DW_OP_swap:
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_tmp = __gdb_stack[__gdb_tos - 1];\n");
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_stack[__gdb_tos - 1] = "
"__gdb_stack[__gdb_tos];\n");
- fprintfi_filtered (indent, &stream, ("__gdb_stack[__gdb_tos] = "
- "__gdb_tmp;\n"));
+ fprintfi_filtered (indent, stream, ("__gdb_stack[__gdb_tos] = "
+ "__gdb_tmp;\n"));
break;
case DW_OP_over:
@@ -929,15 +929,15 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
break;
case DW_OP_rot:
- fprintfi_filtered (indent, &stream, ("__gdb_tmp = "
- "__gdb_stack[__gdb_tos];\n"));
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream, ("__gdb_tmp = "
+ "__gdb_stack[__gdb_tos];\n"));
+ fprintfi_filtered (indent, stream,
"__gdb_stack[__gdb_tos] = "
"__gdb_stack[__gdb_tos - 1];\n");
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_stack[__gdb_tos - 1] = "
"__gdb_stack[__gdb_tos -2];\n");
- fprintfi_filtered (indent, &stream, "__gdb_stack[__gdb_tos - 2] = "
+ fprintfi_filtered (indent, stream, "__gdb_stack[__gdb_tos - 2] = "
"__gdb_tmp;\n");
break;
@@ -959,7 +959,7 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
/* Cast to a pointer of the desired type, then
dereference. */
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_stack[__gdb_tos] = "
"*((__gdb_int_%s *) "
"__gdb_stack[__gdb_tos]);\n",
@@ -1085,19 +1085,19 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
case DW_OP_skip:
offset = extract_signed_integer (op_ptr, 2, byte_order);
op_ptr += 2;
- fprintfi_filtered (indent, &stream, "goto ");
+ fprintfi_filtered (indent, stream, "goto ");
print_label (stream, scope, op_ptr + offset - base);
- stream.puts (";\n");
+ stream->puts (";\n");
break;
case DW_OP_bra:
offset = extract_signed_integer (op_ptr, 2, byte_order);
op_ptr += 2;
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"if ((( " GCC_INTPTR
") __gdb_stack[__gdb_tos--]) != 0) goto ");
print_label (stream, scope, op_ptr + offset - base);
- stream.puts (";\n");
+ stream->puts (";\n");
break;
case DW_OP_nop:
@@ -1108,15 +1108,15 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
}
}
- fprintfi_filtered (indent, &stream, "%s = __gdb_stack[__gdb_tos];\n",
+ fprintfi_filtered (indent, stream, "%s = __gdb_stack[__gdb_tos];\n",
result_name);
- fprintfi_filtered (indent - 2, &stream, "}\n");
+ fprintfi_filtered (indent - 2, stream, "}\n");
}
/* See compile.h. */
void
-compile_dwarf_expr_to_c (string_file &stream, const char *result_name,
+compile_dwarf_expr_to_c (string_file *stream, const char *result_name,
struct symbol *sym, CORE_ADDR pc,
struct gdbarch *arch, unsigned char *registers_used,
unsigned int addr_size,
@@ -1131,7 +1131,7 @@ compile_dwarf_expr_to_c (string_file &stream, const char *result_name,
/* See compile.h. */
void
-compile_dwarf_bounds_to_c (string_file &stream,
+compile_dwarf_bounds_to_c (string_file *stream,
const char *result_name,
const struct dynamic_prop *prop,
struct symbol *sym, CORE_ADDR pc,
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 6d51006..f4d2091 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -141,7 +141,7 @@ compile_instance::compile_instance (struct gcc_base_context *gcc_fe,
/* See compile-internal.h. */
bool
-compile_instance::get_cached_type (struct type *type, gcc_type &ret) const
+compile_instance::get_cached_type (struct type *type, gcc_type *ret) const
{
struct type_map_instance inst, *found;
@@ -149,7 +149,7 @@ compile_instance::get_cached_type (struct type *type, gcc_type &ret) const
found = (struct type_map_instance *) htab_find (m_type_map.get (), &inst);
if (found != NULL)
{
- ret = found->gcc_type_handle;
+ *ret = found->gcc_type_handle;
return true;
}
diff --git a/gdb/compile/compile.h b/gdb/compile/compile.h
index 6e03d7e..89cc00f 100644
--- a/gdb/compile/compile.h
+++ b/gdb/compile/compile.h
@@ -55,7 +55,7 @@ extern void eval_compile_command (struct command_line *cmd,
PER_CU is the per-CU object used for looking up various other
things. */
-extern void compile_dwarf_expr_to_c (string_file &stream,
+extern void compile_dwarf_expr_to_c (string_file *stream,
const char *result_name,
struct symbol *sym,
CORE_ADDR pc,
@@ -90,7 +90,7 @@ extern void compile_dwarf_expr_to_c (string_file &stream,
PER_CU is the per-CU object used for looking up various other
things. */
-extern void compile_dwarf_bounds_to_c (string_file &stream,
+extern void compile_dwarf_bounds_to_c (string_file *stream,
const char *result_name,
const struct dynamic_prop *prop,
struct symbol *sym, CORE_ADDR pc,
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 200fa03..ae5421e 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2747,7 +2747,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
/* See dwarf2loc.h. */
void
-dwarf2_compile_property_to_c (string_file &stream,
+dwarf2_compile_property_to_c (string_file *stream,
const char *result_name,
struct gdbarch *gdbarch,
unsigned char *registers_used,
@@ -4478,7 +4478,7 @@ locexpr_tracepoint_var_ref (struct symbol *symbol, struct agent_expr *ax,
/* symbol_computed_ops 'generate_c_location' method. */
static void
-locexpr_generate_c_location (struct symbol *sym, string_file &stream,
+locexpr_generate_c_location (struct symbol *sym, string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc, const char *result_name)
@@ -4688,7 +4688,7 @@ loclist_tracepoint_var_ref (struct symbol *symbol, struct agent_expr *ax,
/* symbol_computed_ops 'generate_c_location' method. */
static void
-loclist_generate_c_location (struct symbol *sym, string_file &stream,
+loclist_generate_c_location (struct symbol *sym, string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc, const char *result_name)
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index f82e7b2..40b7906 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -156,7 +156,7 @@ int dwarf2_evaluate_property (const struct dynamic_prop *prop,
evaluated.
SYM the originating symbol, used for error reporting. */
-void dwarf2_compile_property_to_c (string_file &stream,
+void dwarf2_compile_property_to_c (string_file *stream,
const char *result_name,
struct gdbarch *gdbarch,
unsigned char *registers_used,
diff --git a/gdb/symtab.h b/gdb/symtab.h
index e0fe17a..399666b 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -983,7 +983,7 @@ struct symbol_computed_ops
The generated C code must assign the location to a local
variable; this variable's name is RESULT_NAME. */
- void (*generate_c_location) (struct symbol *symbol, string_file &stream,
+ void (*generate_c_location) (struct symbol *symbol, string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc, const char *result_name);
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] compile: Remove non-const reference parameters
2018-08-30 16:47 [PATCH] compile: Remove non-const reference parameters Simon Marchi
@ 2018-08-30 21:57 ` Keith Seitz
2018-08-31 14:41 ` Tom Tromey
2018-09-17 6:32 ` Tom Tromey
1 sibling, 1 reply; 11+ messages in thread
From: Keith Seitz @ 2018-08-30 21:57 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 08/30/2018 09:47 AM, Simon Marchi wrote:
>
> I'd say all the changes are pretty obvious, except the one in
> compile_cplus_instance::enter_scope which might require more attention.
That scope-handling code is a little bit of a mess, given that it was written,
and then rewritten, right as we were moving from C -> C++03 -> C++11. So I
had no access to unique_ptrs.
Nonetheless, the intent is that a scope is created and control of that object
is then handed over to the compile_cplus_instance entirely. So your patch
LGTM.
With access to C++11 that we enjoy today, I might have written this
interface quite a bit differently...
Keith
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] compile: Remove non-const reference parameters
2018-08-30 21:57 ` Keith Seitz
@ 2018-08-31 14:41 ` Tom Tromey
2018-09-06 10:27 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2018-08-31 14:41 UTC (permalink / raw)
To: Keith Seitz; +Cc: Simon Marchi, gdb-patches
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
Keith> Nonetheless, the intent is that a scope is created and control of
Keith> that object is then handed over to the compile_cplus_instance
Keith> entirely.
In that case maybe it should take an rvalue ref instead.
I didn't really look at this in context, though, so if that doesn't make
sense, just ignore it.
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] compile: Remove non-const reference parameters
2018-08-31 14:41 ` Tom Tromey
@ 2018-09-06 10:27 ` Simon Marchi
2018-09-06 12:43 ` Tom Tromey
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2018-09-06 10:27 UTC (permalink / raw)
To: Tom Tromey, Keith Seitz; +Cc: gdb-patches
On 2018-08-31 03:41 PM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
>
> Keith> Nonetheless, the intent is that a scope is created and control of
> Keith> that object is then handed over to the compile_cplus_instance
> Keith> entirely.
>
> In that case maybe it should take an rvalue ref instead.
> I didn't really look at this in context, though, so if that doesn't make
> sense, just ignore it.
Good point. How is this? The only change is in enter_scope and its
callers.
From 6a96536caa1c46995b3469fe0248931a04433e80 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Thu, 30 Aug 2018 10:54:45 -0400
Subject: [PATCH] compile: Remove non-const reference parameters
As mentioned here:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Avoid_non-const_reference_parameters.2C_use_pointers_instead
we prefer to avoid non-const references. This patch changes the
non-const references I could find in the compile/ directory, either by
making them rvalue-reference (&&) or changing them to pointers.
I'd say all the changes are pretty obvious, except the one in
compile_cplus_instance::enter_scope which might require more attention.
gdb/ChangeLog:
* compile/compile-c.h (generate_c_for_variable_locations):
Change reference to pointer.
* compile/compile-c-support.c (compile_program) <compute>:
Likewise.
* compile/compile-c-symbols.c (generate_vla_size): Likewise.
(generate_c_for_for_one_variable): Likewise
(generate_c_for_variable_locations): Likewise
* compile/compile-c-types.c (compile_c_instance::convert_type):
Likewise
* compile/compile-cplus-symbols.c (convert_one_symbol):
std::move the scope passed to enter_scope.
* compile/compile-cplus-types.c
(compile_cplus_instance::enter_scope): Make parameter
rvalue-reference.
(compile_cplus_instance::new_scope): Change reference to
pointer.
(compile_cplus_instance::convert_type): Likewise
(compile_cplus_convert_typedef): std::move the scope passed to
enter_scope.
(compile_cplus_convert_struct_or_union): Likewise.
(compile_cplus_convert_enum): Likewise.
(compile_cplus_convert_namespace): Likewise.
* compile/compile-cplus.h (compile_cplus_instance)
<enter_scope>: Make parameter rvalue-reference.
* compile/compile-internal.h (compile_instance)
<get_cached_type>: Likewise
* compile/compile-loc2c.c (push): Likewise
(pushf): Likewise
(unary): Likewise
(binary): Likewise
(print_label): Likewise
(pushf_register_address): Likewise
(pushf_register): Likewise
(do_compile_dwarf_expr_to_c): Likewise
(compile_dwarf_expr_to_c): Likewise
(compile_dwarf_bounds_to_c): Likewise
* compile/compile.c (compile_instance::get_cached_type):
Likewise
* compile/compile.h (compile_dwarf_expr_to_c): Likewise.
(compile_dwarf_bounds_to_c): Likewise
* dwarf2loc.c (locexpr_generate_c_location): Likewise.
(dwarf2_compile_property_to_c): Likewise
* dwarf2loc.h (dwarf2_compile_property_to_c): Likewise
* symtab.h (struct symbol_computed_ops) <generate_c_location>:
Likewise
---
gdb/compile/compile-c-support.c | 2 +-
gdb/compile/compile-c-symbols.c | 14 ++---
gdb/compile/compile-c-types.c | 2 +-
gdb/compile/compile-c.h | 2 +-
gdb/compile/compile-cplus-symbols.c | 6 +-
gdb/compile/compile-cplus-types.c | 16 +++---
gdb/compile/compile-cplus.h | 2 +-
gdb/compile/compile-internal.h | 2 +-
gdb/compile/compile-loc2c.c | 106 ++++++++++++++++++------------------
gdb/compile/compile.c | 4 +-
gdb/compile/compile.h | 4 +-
gdb/dwarf2loc.c | 6 +-
gdb/dwarf2loc.h | 2 +-
gdb/symtab.h | 2 +-
14 files changed, 84 insertions(+), 86 deletions(-)
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 5c700bb..28a07dc 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -572,7 +572,7 @@ public:
register struct before the function body. This requires a
temporary stream. */
gdb::unique_xmalloc_ptr<unsigned char> registers_used
- = generate_c_for_variable_locations (m_instance, var_stream, m_arch,
+ = generate_c_for_variable_locations (m_instance, &var_stream, m_arch,
expr_block, expr_pc);
buf.puts ("typedef unsigned int"
diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index e7423d1..e877b78 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -487,7 +487,7 @@ symbol_seen (htab_t hashtab, struct symbol *sym)
static void
generate_vla_size (compile_instance *compiler,
- string_file &stream,
+ string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc,
@@ -541,7 +541,7 @@ generate_vla_size (compile_instance *compiler,
static void
generate_c_for_for_one_variable (compile_instance *compiler,
- string_file &stream,
+ string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc,
@@ -556,10 +556,10 @@ generate_c_for_for_one_variable (compile_instance *compiler,
occurs in the middle. */
string_file local_file;
- generate_vla_size (compiler, local_file, gdbarch, registers_used, pc,
+ generate_vla_size (compiler, &local_file, gdbarch, registers_used, pc,
SYMBOL_TYPE (sym), sym);
- stream.write (local_file.c_str (), local_file.size ());
+ stream->write (local_file.c_str (), local_file.size ());
}
if (SYMBOL_COMPUTED_OPS (sym) != NULL)
@@ -570,12 +570,12 @@ generate_c_for_for_one_variable (compile_instance *compiler,
occurs in the middle. */
string_file local_file;
- SYMBOL_COMPUTED_OPS (sym)->generate_c_location (sym, local_file,
+ SYMBOL_COMPUTED_OPS (sym)->generate_c_location (sym, &local_file,
gdbarch,
registers_used,
pc,
generated_name.get ());
- stream.write (local_file.c_str (), local_file.size ());
+ stream->write (local_file.c_str (), local_file.size ());
}
else
{
@@ -611,7 +611,7 @@ generate_c_for_for_one_variable (compile_instance *compiler,
gdb::unique_xmalloc_ptr<unsigned char>
generate_c_for_variable_locations (compile_instance *compiler,
- string_file &stream,
+ string_file *stream,
struct gdbarch *gdbarch,
const struct block *block,
CORE_ADDR pc)
diff --git a/gdb/compile/compile-c-types.c b/gdb/compile/compile-c-types.c
index 30a4fcb..33b4a87 100644
--- a/gdb/compile/compile-c-types.c
+++ b/gdb/compile/compile-c-types.c
@@ -353,7 +353,7 @@ compile_c_instance::convert_type (struct type *type)
type = check_typedef (type);
gcc_type result;
- if (get_cached_type (type, result))
+ if (get_cached_type (type, &result))
return result;
result = convert_type_basic (this, type);
diff --git a/gdb/compile/compile-c.h b/gdb/compile/compile-c.h
index 18ff4d3..aa11a13 100644
--- a/gdb/compile/compile-c.h
+++ b/gdb/compile/compile-c.h
@@ -69,7 +69,7 @@ private:
extern gdb::unique_xmalloc_ptr<unsigned char>
generate_c_for_variable_locations
(compile_instance *compiler,
- string_file &stream,
+ string_file *stream,
struct gdbarch *gdbarch,
const struct block *block,
CORE_ADDR pc);
diff --git a/gdb/compile/compile-cplus-symbols.c b/gdb/compile/compile-cplus-symbols.c
index 234b977..6acbd9a 100644
--- a/gdb/compile/compile-cplus-symbols.c
+++ b/gdb/compile/compile-cplus-symbols.c
@@ -184,13 +184,11 @@ convert_one_symbol (compile_cplus_instance *instance,
/* Don't emit local variable decls for a raw expression. */
if (instance->scope () != COMPILE_I_RAW_SCOPE || symbol_name == nullptr)
{
- compile_scope scope;
-
/* For non-local symbols, create/push a new scope so that the
symbol is properly scoped to the plug-in. */
if (!is_local)
{
- scope
+ compile_scope scope
= instance->new_scope (SYMBOL_NATURAL_NAME (sym.symbol),
SYMBOL_TYPE (sym.symbol));
if (scope.nested_type () != GCC_TYPE_NONE)
@@ -200,7 +198,7 @@ convert_one_symbol (compile_cplus_instance *instance,
return;
}
- instance->enter_scope (scope);
+ instance->enter_scope (std::move (scope));
}
/* Get the `raw' name of the symbol. */
diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index 271cc66..75193d2 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -247,14 +247,14 @@ operator!= (const compile_scope &lhs, const compile_scope &rhs)
/* See description in compile-cplus.h. */
void
-compile_cplus_instance::enter_scope (compile_scope &new_scope)
+compile_cplus_instance::enter_scope (compile_scope &&new_scope)
{
bool must_push = m_scopes.empty () || m_scopes.back () != new_scope;
new_scope.m_pushed = must_push;
/* Save the new scope. */
- m_scopes.push_back (new_scope);
+ m_scopes.push_back (std::move (new_scope));
if (must_push)
{
@@ -360,7 +360,7 @@ compile_cplus_instance::new_scope (const char *type_name, struct type *type)
class, the previous call will give us that type's gcc_type.
Upper layers are expecting to get the original type's
gcc_type! */
- get_cached_type (type, scope.m_nested_type);
+ get_cached_type (type, &scope.m_nested_type);
return scope;
}
}
@@ -526,7 +526,7 @@ compile_cplus_convert_typedef (compile_cplus_instance *instance,
= compile_cplus_instance::decl_name (TYPE_NAME (type));
/* Make sure the scope for this type has been pushed. */
- instance->enter_scope (scope);
+ instance->enter_scope (std::move (scope));
/* Convert the typedef's real type. */
gcc_type typedef_type = instance->convert_type (check_typedef (type));
@@ -822,7 +822,7 @@ compile_cplus_convert_struct_or_union (compile_cplus_instance *instance,
}
/* Push all scopes. */
- instance->enter_scope (scope);
+ instance->enter_scope (std::move (scope));
/* First we create the resulting type and enter it into our hash
table. This lets recursive types work. */
@@ -928,7 +928,7 @@ compile_cplus_convert_enum (compile_cplus_instance *instance, struct type *type,
= compile_cplus_instance::decl_name (TYPE_NAME (type));
/* Push all scopes. */
- instance->enter_scope (scope);
+ instance->enter_scope (std::move (scope));
gcc_type int_type
= instance->plugin ().get_int_type (TYPE_UNSIGNED (type),
@@ -1109,7 +1109,7 @@ compile_cplus_convert_namespace (compile_cplus_instance *instance,
= compile_cplus_instance::decl_name (TYPE_NAME (type));
/* Push scope. */
- instance->enter_scope (scope);
+ instance->enter_scope (std::move (scope));
/* Convert this namespace. */
instance->plugin ().push_namespace (name.get ());
@@ -1211,7 +1211,7 @@ compile_cplus_instance::convert_type (struct type *type,
{
/* Check if TYPE has already been converted. */
gcc_type result;
- if (get_cached_type (type, result))
+ if (get_cached_type (type, &result))
return result;
/* It is the first time this type has been seen -- convert it
diff --git a/gdb/compile/compile-cplus.h b/gdb/compile/compile-cplus.h
index 7baa57d..040f0cc 100644
--- a/gdb/compile/compile-cplus.h
+++ b/gdb/compile/compile-cplus.h
@@ -159,7 +159,7 @@ public:
compile_scope new_scope (const char *type_name, struct type *type);
/* Enter the given NEW_SCOPE. */
- void enter_scope (compile_scope &scope);
+ void enter_scope (compile_scope &&scope);
/* Leave the current scope. */
void leave_scope ();
diff --git a/gdb/compile/compile-internal.h b/gdb/compile/compile-internal.h
index a6e7330..2df8b89 100644
--- a/gdb/compile/compile-internal.h
+++ b/gdb/compile/compile-internal.h
@@ -59,7 +59,7 @@ public:
/* Query the type cache for TYPE, returning the compiler's
type for it in RET. */
- bool get_cached_type (struct type *type, gcc_type &ret) const;
+ bool get_cached_type (struct type *type, gcc_type *ret) const;
/* Insert GCC_TYPE into the type cache for TYPE.
diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
index bd080f8..1997e9a 100644
--- a/gdb/compile/compile-loc2c.c
+++ b/gdb/compile/compile-loc2c.c
@@ -430,9 +430,9 @@ compute_stack_depth (enum bfd_endian byte_order, unsigned int addr_size,
/* Emit code to push a constant. */
static void
-push (int indent, string_file &stream, ULONGEST l)
+push (int indent, string_file *stream, ULONGEST l)
{
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_stack[++__gdb_tos] = (" GCC_UINTPTR ") %s;\n",
hex_string (l));
}
@@ -440,57 +440,57 @@ push (int indent, string_file &stream, ULONGEST l)
/* Emit code to push an arbitrary expression. This works like
printf. */
-static void pushf (int indent, string_file &stream, const char *format, ...)
+static void pushf (int indent, string_file *stream, const char *format, ...)
ATTRIBUTE_PRINTF (3, 4);
static void
-pushf (int indent, string_file &stream, const char *format, ...)
+pushf (int indent, string_file *stream, const char *format, ...)
{
va_list args;
- fprintfi_filtered (indent, &stream, "__gdb_stack[__gdb_tos + 1] = ");
+ fprintfi_filtered (indent, stream, "__gdb_stack[__gdb_tos + 1] = ");
va_start (args, format);
- stream.vprintf (format, args);
+ stream->vprintf (format, args);
va_end (args);
- stream.puts (";\n");
+ stream->puts (";\n");
- fprintfi_filtered (indent, &stream, "++__gdb_tos;\n");
+ fprintfi_filtered (indent, stream, "++__gdb_tos;\n");
}
/* Emit code for a unary expression -- one which operates in-place on
the top-of-stack. This works like printf. */
-static void unary (int indent, string_file &stream, const char *format, ...)
+static void unary (int indent, string_file *stream, const char *format, ...)
ATTRIBUTE_PRINTF (3, 4);
static void
-unary (int indent, string_file &stream, const char *format, ...)
+unary (int indent, string_file *stream, const char *format, ...)
{
va_list args;
- fprintfi_filtered (indent, &stream, "__gdb_stack[__gdb_tos] = ");
+ fprintfi_filtered (indent, stream, "__gdb_stack[__gdb_tos] = ");
va_start (args, format);
- stream.vprintf (format, args);
+ stream->vprintf (format, args);
va_end (args);
- stream.puts (";\n");
+ stream->puts (";\n");
}
/* Emit code for a unary expression -- one which uses the top two
stack items, popping the topmost one. This works like printf. */
-static void binary (int indent, string_file &stream, const char *format, ...)
+static void binary (int indent, string_file *stream, const char *format, ...)
ATTRIBUTE_PRINTF (3, 4);
static void
-binary (int indent, string_file &stream, const char *format, ...)
+binary (int indent, string_file *stream, const char *format, ...)
{
va_list args;
- fprintfi_filtered (indent, &stream, "__gdb_stack[__gdb_tos - 1] = ");
+ fprintfi_filtered (indent, stream, "__gdb_stack[__gdb_tos - 1] = ");
va_start (args, format);
- stream.vprintf (format, args);
+ stream->vprintf (format, args);
va_end (args);
- stream.puts (";\n");
- fprintfi_filtered (indent, &stream, "--__gdb_tos;\n");
+ stream->puts (";\n");
+ fprintfi_filtered (indent, stream, "--__gdb_tos;\n");
}
/* Print the name of a label given its "SCOPE", an arbitrary integer
@@ -498,9 +498,9 @@ binary (int indent, string_file &stream, const char *format, ...)
corresponding to the label's point of definition. */
static void
-print_label (string_file &stream, unsigned int scope, int target)
+print_label (string_file *stream, unsigned int scope, int target)
{
- stream.printf ("__label_%u_%s", scope, pulongest (target));
+ stream->printf ("__label_%u_%s", scope, pulongest (target));
}
/* Emit code that pushes a register's address on the stack.
@@ -508,7 +508,7 @@ print_label (string_file &stream, unsigned int scope, int target)
register was needed by this expression. */
static void
-pushf_register_address (int indent, string_file &stream,
+pushf_register_address (int indent, string_file *stream,
unsigned char *registers_used,
struct gdbarch *gdbarch, int regnum)
{
@@ -526,7 +526,7 @@ pushf_register_address (int indent, string_file &stream,
register's value before it is pushed. */
static void
-pushf_register (int indent, string_file &stream,
+pushf_register (int indent, string_file *stream,
unsigned char *registers_used,
struct gdbarch *gdbarch, int regnum, uint64_t offset)
{
@@ -572,7 +572,7 @@ pushf_register (int indent, string_file &stream,
things. */
static void
-do_compile_dwarf_expr_to_c (int indent, string_file &stream,
+do_compile_dwarf_expr_to_c (int indent, string_file *stream,
const char *type_name,
const char *result_name,
struct symbol *sym, CORE_ADDR pc,
@@ -596,9 +596,9 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
++scope;
- fprintfi_filtered (indent, &stream, "__attribute__ ((unused)) %s %s;\n",
+ fprintfi_filtered (indent, stream, "__attribute__ ((unused)) %s %s;\n",
type_name, result_name);
- fprintfi_filtered (indent, &stream, "{\n");
+ fprintfi_filtered (indent, stream, "{\n");
indent += 2;
stack_depth = compute_stack_depth (byte_order, addr_size,
@@ -634,19 +634,19 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
"compiled code."),
SYMBOL_PRINT_NAME (sym));
- fprintfi_filtered (indent, &stream, "%s = %s;\n",
+ fprintfi_filtered (indent, stream, "%s = %s;\n",
result_name,
core_addr_to_string (value_address (val)));
- fprintfi_filtered (indent - 2, &stream, "}\n");
+ fprintfi_filtered (indent - 2, stream, "}\n");
return;
}
- fprintfi_filtered (indent, &stream, GCC_UINTPTR " __gdb_stack[%d];\n",
+ fprintfi_filtered (indent, stream, GCC_UINTPTR " __gdb_stack[%d];\n",
stack_depth);
if (need_tempvar)
- fprintfi_filtered (indent, &stream, GCC_UINTPTR " __gdb_tmp;\n");
- fprintfi_filtered (indent, &stream, "int __gdb_tos = -1;\n");
+ fprintfi_filtered (indent, stream, GCC_UINTPTR " __gdb_tmp;\n");
+ fprintfi_filtered (indent, stream, "int __gdb_tos = -1;\n");
if (initial != NULL)
pushf (indent, stream, "%s", core_addr_to_string (*initial));
@@ -657,13 +657,13 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
uint64_t uoffset, reg;
int64_t offset;
- print_spaces (indent - 2, &stream);
+ print_spaces (indent - 2, stream);
if (info[op_ptr - base].label)
{
print_label (stream, scope, op_ptr - base);
- stream.puts (":;");
+ stream->puts (":;");
}
- stream.printf ("/* %s */\n", get_DW_OP_name (op));
+ stream->printf ("/* %s */\n", get_DW_OP_name (op));
/* This is handy for debugging the generated code:
fprintf_filtered (stream, "if (__gdb_tos != %d) abort ();\n",
@@ -905,7 +905,7 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
break;
case DW_OP_drop:
- fprintfi_filtered (indent, &stream, "--__gdb_tos;\n");
+ fprintfi_filtered (indent, stream, "--__gdb_tos;\n");
break;
case DW_OP_pick:
@@ -915,13 +915,13 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
break;
case DW_OP_swap:
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_tmp = __gdb_stack[__gdb_tos - 1];\n");
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_stack[__gdb_tos - 1] = "
"__gdb_stack[__gdb_tos];\n");
- fprintfi_filtered (indent, &stream, ("__gdb_stack[__gdb_tos] = "
- "__gdb_tmp;\n"));
+ fprintfi_filtered (indent, stream, ("__gdb_stack[__gdb_tos] = "
+ "__gdb_tmp;\n"));
break;
case DW_OP_over:
@@ -929,15 +929,15 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
break;
case DW_OP_rot:
- fprintfi_filtered (indent, &stream, ("__gdb_tmp = "
- "__gdb_stack[__gdb_tos];\n"));
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream, ("__gdb_tmp = "
+ "__gdb_stack[__gdb_tos];\n"));
+ fprintfi_filtered (indent, stream,
"__gdb_stack[__gdb_tos] = "
"__gdb_stack[__gdb_tos - 1];\n");
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_stack[__gdb_tos - 1] = "
"__gdb_stack[__gdb_tos -2];\n");
- fprintfi_filtered (indent, &stream, "__gdb_stack[__gdb_tos - 2] = "
+ fprintfi_filtered (indent, stream, "__gdb_stack[__gdb_tos - 2] = "
"__gdb_tmp;\n");
break;
@@ -959,7 +959,7 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
/* Cast to a pointer of the desired type, then
dereference. */
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"__gdb_stack[__gdb_tos] = "
"*((__gdb_int_%s *) "
"__gdb_stack[__gdb_tos]);\n",
@@ -1085,19 +1085,19 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
case DW_OP_skip:
offset = extract_signed_integer (op_ptr, 2, byte_order);
op_ptr += 2;
- fprintfi_filtered (indent, &stream, "goto ");
+ fprintfi_filtered (indent, stream, "goto ");
print_label (stream, scope, op_ptr + offset - base);
- stream.puts (";\n");
+ stream->puts (";\n");
break;
case DW_OP_bra:
offset = extract_signed_integer (op_ptr, 2, byte_order);
op_ptr += 2;
- fprintfi_filtered (indent, &stream,
+ fprintfi_filtered (indent, stream,
"if ((( " GCC_INTPTR
") __gdb_stack[__gdb_tos--]) != 0) goto ");
print_label (stream, scope, op_ptr + offset - base);
- stream.puts (";\n");
+ stream->puts (";\n");
break;
case DW_OP_nop:
@@ -1108,15 +1108,15 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
}
}
- fprintfi_filtered (indent, &stream, "%s = __gdb_stack[__gdb_tos];\n",
+ fprintfi_filtered (indent, stream, "%s = __gdb_stack[__gdb_tos];\n",
result_name);
- fprintfi_filtered (indent - 2, &stream, "}\n");
+ fprintfi_filtered (indent - 2, stream, "}\n");
}
/* See compile.h. */
void
-compile_dwarf_expr_to_c (string_file &stream, const char *result_name,
+compile_dwarf_expr_to_c (string_file *stream, const char *result_name,
struct symbol *sym, CORE_ADDR pc,
struct gdbarch *arch, unsigned char *registers_used,
unsigned int addr_size,
@@ -1131,7 +1131,7 @@ compile_dwarf_expr_to_c (string_file &stream, const char *result_name,
/* See compile.h. */
void
-compile_dwarf_bounds_to_c (string_file &stream,
+compile_dwarf_bounds_to_c (string_file *stream,
const char *result_name,
const struct dynamic_prop *prop,
struct symbol *sym, CORE_ADDR pc,
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 6d51006..f4d2091 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -141,7 +141,7 @@ compile_instance::compile_instance (struct gcc_base_context *gcc_fe,
/* See compile-internal.h. */
bool
-compile_instance::get_cached_type (struct type *type, gcc_type &ret) const
+compile_instance::get_cached_type (struct type *type, gcc_type *ret) const
{
struct type_map_instance inst, *found;
@@ -149,7 +149,7 @@ compile_instance::get_cached_type (struct type *type, gcc_type &ret) const
found = (struct type_map_instance *) htab_find (m_type_map.get (), &inst);
if (found != NULL)
{
- ret = found->gcc_type_handle;
+ *ret = found->gcc_type_handle;
return true;
}
diff --git a/gdb/compile/compile.h b/gdb/compile/compile.h
index 6e03d7e..89cc00f 100644
--- a/gdb/compile/compile.h
+++ b/gdb/compile/compile.h
@@ -55,7 +55,7 @@ extern void eval_compile_command (struct command_line *cmd,
PER_CU is the per-CU object used for looking up various other
things. */
-extern void compile_dwarf_expr_to_c (string_file &stream,
+extern void compile_dwarf_expr_to_c (string_file *stream,
const char *result_name,
struct symbol *sym,
CORE_ADDR pc,
@@ -90,7 +90,7 @@ extern void compile_dwarf_expr_to_c (string_file &stream,
PER_CU is the per-CU object used for looking up various other
things. */
-extern void compile_dwarf_bounds_to_c (string_file &stream,
+extern void compile_dwarf_bounds_to_c (string_file *stream,
const char *result_name,
const struct dynamic_prop *prop,
struct symbol *sym, CORE_ADDR pc,
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 84e3c3c..1c21895 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2749,7 +2749,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
/* See dwarf2loc.h. */
void
-dwarf2_compile_property_to_c (string_file &stream,
+dwarf2_compile_property_to_c (string_file *stream,
const char *result_name,
struct gdbarch *gdbarch,
unsigned char *registers_used,
@@ -4480,7 +4480,7 @@ locexpr_tracepoint_var_ref (struct symbol *symbol, struct agent_expr *ax,
/* symbol_computed_ops 'generate_c_location' method. */
static void
-locexpr_generate_c_location (struct symbol *sym, string_file &stream,
+locexpr_generate_c_location (struct symbol *sym, string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc, const char *result_name)
@@ -4690,7 +4690,7 @@ loclist_tracepoint_var_ref (struct symbol *symbol, struct agent_expr *ax,
/* symbol_computed_ops 'generate_c_location' method. */
static void
-loclist_generate_c_location (struct symbol *sym, string_file &stream,
+loclist_generate_c_location (struct symbol *sym, string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc, const char *result_name)
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index d02e3cd..d7a56db 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -156,7 +156,7 @@ int dwarf2_evaluate_property (const struct dynamic_prop *prop,
evaluated.
SYM the originating symbol, used for error reporting. */
-void dwarf2_compile_property_to_c (string_file &stream,
+void dwarf2_compile_property_to_c (string_file *stream,
const char *result_name,
struct gdbarch *gdbarch,
unsigned char *registers_used,
diff --git a/gdb/symtab.h b/gdb/symtab.h
index e0fe17a..399666b 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -983,7 +983,7 @@ struct symbol_computed_ops
The generated C code must assign the location to a local
variable; this variable's name is RESULT_NAME. */
- void (*generate_c_location) (struct symbol *symbol, string_file &stream,
+ void (*generate_c_location) (struct symbol *symbol, string_file *stream,
struct gdbarch *gdbarch,
unsigned char *registers_used,
CORE_ADDR pc, const char *result_name);
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] compile: Remove non-const reference parameters
2018-09-06 10:27 ` Simon Marchi
@ 2018-09-06 12:43 ` Tom Tromey
2018-09-06 12:49 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2018-09-06 12:43 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, Keith Seitz, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> Good point. How is this? The only change is in enter_scope and its
Simon> callers.
Looks reasonable to me.
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] compile: Remove non-const reference parameters
2018-09-06 12:43 ` Tom Tromey
@ 2018-09-06 12:49 ` Simon Marchi
0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2018-09-06 12:49 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simon Marchi, Keith Seitz, gdb-patches
On 2018-09-06 13:42, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> Good point. How is this? The only change is in enter_scope and
> its
> Simon> callers.
>
> Looks reasonable to me.
>
> Tom
Thanks, I pushed it.
Simon
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] compile: Remove non-const reference parameters
2018-08-30 16:47 [PATCH] compile: Remove non-const reference parameters Simon Marchi
2018-08-30 21:57 ` Keith Seitz
@ 2018-09-17 6:32 ` Tom Tromey
2018-09-17 13:29 ` Keith Seitz
1 sibling, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2018-09-17 6:32 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, Keith Seitz
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> As mentioned here:
Simon> https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Avoid_non-const_reference_parameters.2C_use_pointers_instead
Simon> we prefer to avoid non-const references. This patch changes the
Simon> non-const references I could find in the compile/ directory, either by
Simon> making them const or changing them to pointers.
Simon> I'd say all the changes are pretty obvious, except the one in
Simon> compile_cplus_instance::enter_scope which might require more attention.
I think this particular change introduced a regression. runtest
gdb.compile/*.exp causes gdb to crash for me several times on x86-64
Fedora 28.
The code does this:
/* Save the new scope. */
m_scopes.push_back (std::move (new_scope));
... but then later code in the function continues to use new_scope,
like:
std::for_each
(new_scope.begin (), new_scope.end () - 1,
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] compile: Remove non-const reference parameters
2018-09-17 6:32 ` Tom Tromey
@ 2018-09-17 13:29 ` Keith Seitz
2018-09-17 13:52 ` [PATCH] Fix use-after-move in compile/compile-cplus-types.c Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Keith Seitz @ 2018-09-17 13:29 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi; +Cc: gdb-patches
On 09/16/2018 11:31 PM, Tom Tromey wrote:
> I think this particular change introduced a regression. runtest
> gdb.compile/*.exp causes gdb to crash for me several times on x86-64
> Fedora 28.
Fedora 28 is really broken right now. I have patches to fix almost
everything*, but I am struggling to understand why one of them is needed.
AFAICT, either __builtin_memcpy has usage changes that I missed, or there is
a glibc bug here. ["compile print" is completely broken -- the
__builtin_memcpy on Fedora 28 does nothing. Works fine on my Fedora 21
and 26 boxes.]
Using __builtin_memmove works fine, though. [And just to state the obvious,
no, the memory regions do not overlap.]
Keith
* "Everything" = everything except what appears to be a newly introduced GCC
8 bug that causes regressions in compile-cplus-method.exp.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Fix use-after-move in compile/compile-cplus-types.c
2018-09-17 13:29 ` Keith Seitz
@ 2018-09-17 13:52 ` Simon Marchi
2018-09-17 15:25 ` Keith Seitz
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2018-09-17 13:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
Patch
d82b3862f12 ("compile: Remove non-const reference parameters")
introduced a regression in compile/compile-cplus-types.c. The new_scope
variable in compile_cplus_instance::enter_scope is used after it was
std::moved. This patch fixes it by referring to the back of the vector
where it was moved instead.
gdb/ChangeLog:
* compile/compile-cplus-types.c
(compile_cplus_instance::enter_scope): Don't use new_scope after
std::move.
---
gdb/compile/compile-cplus-types.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index 75193d2e75b..996fea56986 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -261,7 +261,7 @@ compile_cplus_instance::enter_scope (compile_scope &&new_scope)
if (debug_compile_cplus_scopes)
{
fprintf_unfiltered (gdb_stdlog, "entering new scope %s\n",
- host_address_to_string (&new_scope));
+ host_address_to_string (&m_scopes.back ()));
}
/* Push the global namespace. */
@@ -270,7 +270,7 @@ compile_cplus_instance::enter_scope (compile_scope &&new_scope)
/* Push all other namespaces. Note that we do not push the last
scope_component -- that's the actual type we are converting. */
std::for_each
- (new_scope.begin (), new_scope.end () - 1,
+ (m_scopes.back ().begin (), m_scopes.back ().end () - 1,
[this] (const scope_component &comp)
{
gdb_assert (TYPE_CODE (SYMBOL_TYPE (comp.bsymbol.symbol))
--
2.19.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix use-after-move in compile/compile-cplus-types.c
2018-09-17 13:52 ` [PATCH] Fix use-after-move in compile/compile-cplus-types.c Simon Marchi
@ 2018-09-17 15:25 ` Keith Seitz
2018-09-17 17:10 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Keith Seitz @ 2018-09-17 15:25 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 09/17/2018 06:52 AM, Simon Marchi wrote:
> Patch
>
> d82b3862f12 ("compile: Remove non-const reference parameters")
>
> introduced a regression in compile/compile-cplus-types.c. The new_scope
> variable in compile_cplus_instance::enter_scope is used after it was
> std::moved. This patch fixes it by referring to the back of the vector
> where it was moved instead.
>
> gdb/ChangeLog:
>
> * compile/compile-cplus-types.c
> (compile_cplus_instance::enter_scope): Don't use new_scope after
> std::move.
That LGTM. [Although I would have used a const reference to it everywhere, but
six of one, ...]
Keith
PS. Reminder: IANAM, but you are. So please approve your patch. :-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix use-after-move in compile/compile-cplus-types.c
2018-09-17 15:25 ` Keith Seitz
@ 2018-09-17 17:10 ` Simon Marchi
0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2018-09-17 17:10 UTC (permalink / raw)
To: Keith Seitz; +Cc: Simon Marchi, gdb-patches
On 2018-09-17 11:25, Keith Seitz wrote:
> On 09/17/2018 06:52 AM, Simon Marchi wrote:
>> Patch
>>
>> d82b3862f12 ("compile: Remove non-const reference parameters")
>>
>> introduced a regression in compile/compile-cplus-types.c. The
>> new_scope
>> variable in compile_cplus_instance::enter_scope is used after it was
>> std::moved. This patch fixes it by referring to the back of the
>> vector
>> where it was moved instead.
>>
>> gdb/ChangeLog:
>>
>> * compile/compile-cplus-types.c
>> (compile_cplus_instance::enter_scope): Don't use new_scope after
>> std::move.
>
> That LGTM. [Although I would have used a const reference to it
> everywhere, but
> six of one, ...]
Yeah, I thought about that too, but then I didn't know what to name the
new variable, since "new_scope" is already taken. So in the end I chose
the solution where I didn't have to choose a name :).
> PS. Reminder: IANAM, but you are. So please approve your patch. :-)
Thanks for taking a look, I am pushing it.
Simon
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-09-17 17:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-30 16:47 [PATCH] compile: Remove non-const reference parameters Simon Marchi
2018-08-30 21:57 ` Keith Seitz
2018-08-31 14:41 ` Tom Tromey
2018-09-06 10:27 ` Simon Marchi
2018-09-06 12:43 ` Tom Tromey
2018-09-06 12:49 ` Simon Marchi
2018-09-17 6:32 ` Tom Tromey
2018-09-17 13:29 ` Keith Seitz
2018-09-17 13:52 ` [PATCH] Fix use-after-move in compile/compile-cplus-types.c Simon Marchi
2018-09-17 15:25 ` Keith Seitz
2018-09-17 17:10 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox