Re: [PATCH 9/9] C++ compile support

[Tom Tromey <tom@tromey.com>]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> This patch adds *basic* support for C++ to the compile feature.  It does
Keith> most simple type conversions, including everything that C compile does and
Keith> your basic "with-classes" type of C++.

Thanks for doing this.  This is very good.

Keith> +extern compile_instance *cplus_get_compile_context (void);

I think we stopped using (void) now.

Keith> +typedef compile_program<compile_cplus_instance,
Keith> +			cplus_push_user_expression, cplus_pop_user_expression,
Keith> +			cplus_add_code_header, c_add_code_footer,
Keith> +			cplus_add_input> cplus_compile_program;
Keith> +

Haha, this seemed like overkill to me, but I see it's already this way,
and it doesn't really matter I think.

Keith> diff --git a/gdb/compile/compile-cplus-support.c b/gdb/compile/compile-cplus-support.c
Keith> new file mode 100644
Keith> index 0000000000..42fc7692c4
[...]
Keith> +void
Keith> +gcc_cplus_enter_scope (void *datum, struct gcc_cp_context *gcc_context)
Keith> +{
Keith> +}
Keith> +
Keith> +void
Keith> +gcc_cplus_leave_scope (void *datum, struct gcc_cp_context *gcc_context)
Keith> +{
Keith> +}

Is this something that gets expanded in later patches?

If not, I'd suggest just making these private static methods on
compile_cplus_instance, to avoid needing a new file.

On the other hand, if this is expanded in the future, then it is totally
fine like this.

Keith> +/* Convert a given symbol, SYM, to the compiler's representation.
Keith> +   INSTANCE is the compiler instance.  IS_GLOBAL is true if the
Keith> +   symbol came from the global scope.  IS_LOCAL is true if the symbol
Keith> +   came from a local scope.  (Note that the two are not strictly
Keith> +   inverses because the symbol might have come from the static
Keith> +   scope.)  */
Keith> +
Keith> +static void
Keith> +convert_one_symbol (compile_cplus_instance *instance,
Keith> +		    struct block_symbol sym, bool is_global, bool is_local)

I think back in the day, I'd hoped that there would be more code sharing
between the C and C++ compile paths.  But maybe this wasn't possible?  I
didn't do a deep comparison or anything.  And it's fine if they have to
be separate.

Keith> +void _initialize_compile_cplus_symbols (void);

I think this declaration isn't needed any more.

Keith> +/* See description in compile-cplus.h.  */
Keith> +
Keith> +gdb::unique_xmalloc_ptr<char>
Keith> +decl_name (const char *natural)

This should return a std::string.  See below.

Keith> +      std::string symbol = (comp.bsymbol.symbol != nullptr
Keith> +			    ? SYMBOL_NATURAL_NAME (comp.bsymbol.symbol)
Keith> +			    : "<none>");

This could just be a const char * to avoid an allocation.

Keith> +/* Utility function to convert CODE into a string.  */
Keith> +
Keith> +static const char *
Keith> +type_code_to_string (enum type_code code)
Keith> +{
Keith> +  const char * const s[] =
Keith> +    {"BISTRING (deprecated)", "UNDEF (not used)",
Keith> +     "PTR", "ARRAY", "STRUCT", "UNION", "ENUM",
Keith> +     "FLAGS", "FUNC", "INT", "FLT", "VOID",
Keith> +     "SET", "RANGE", "STRING", "ERROR", "METHOD",
Keith> +     "METHODPTR", "MEMBERPTR", "REF", "RVALUE_REF", "CHAR", "BOOL",
Keith> +     "COMPLEX", "TYPEDEF", "NAMESPACE", "DECFLOAT", "MODULE",
Keith> +     "INTERNAL_FUNCTION", "XMETHOD"};
Keith> +
Keith> +  return s[code + 1];

This could get out of sync with gdbtypes.h.  So I'd suggest either (1)
taking the simple route and printing this as an integer; or (2) moving
the enum to a .defs file and auto-generating the strings from it, to
avoid any possibility of problems.  In case 2, type_code_to_string could
move to gdbtypes.c.

Keith> +	  scope_component comp
Keith> +	    = {
Keith> +	        decl_name (TYPE_NAME (type)).release (),
Keith> +		lookup_symbol (TYPE_NAME (type), block (), VAR_DOMAIN, nullptr)

scope_component::name is a std::string, so I think this decl_name call
will leak memory; which is why decl_name should return a std::string
itself.

Keith> +/* Return the declaration name of the symbol named NATURAL.
Keith> +   This returns a name with no function arguments or template parameters,
Keith> +   suitable for passing to the compiler plug-in.  */
Keith> +
Keith> +gdb::unique_xmalloc_ptr<char> decl_name (const char *natural);

This seems like a fairly generic name for something that is maybe
specific to the C++ compile code.

Tom