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

Keith Seitz
2018-08-17T17:51:01Z

On 08/11/2018 12:22 AM, Eli Zaretskii wrote:
>> From: Keith Seitz <keiths@redhat.com>
>> Date: Fri, 10 Aug 2018 16:25:34 -0700
>>
>> This initial support has several glaring omissions:
>> - No template support at all
>> I have follow-on patches for this, but they add much complexity
>> to this "basic" support. Consequently, they will be submitted separately.
>> - Cannot print functions
>> The code template needs tweaking, and I simply haven't gotten to it yet.
>> - So-called "special function" support is not included
>> Using constructors, destructors, operators, etc will not work. I have
>> follow-on patches for that, but they require some work because of the
>> recent churn in symbol searching.
>> - There are several test suite references to "compile/1234" bugs.
>> I will file bugs and update the test suite's bug references before pushing
>> these patches.
>
> Shouldn't these omissions be mentioned somewhere? Like in NEWS?

To be honest, I was hesitant to even mention this in NEWS at all (yet).
The code is *very* beta and incomplete (as you can see). That's why the
"experimental" moniker. Nonetheless, assuming we want to mention it, I've
changed the entry to:

* GDB now has experimental support for the compilation and injection of
C++ source code into the inferior. This beta release does not include
support for several language features, such as templates, constructors,
and operators.

>
>> +* GDB now has experimental support for the compilation and injection of
>> + C++ source code into the inferior. This feature requires the GCC C++
>> + plug-in available since GCC 7.1.
>
> The last sentence basically means "only on ELF platforms", right? So
> IMO this limitation should be stated explicitly, lest people get
> excited, and then get disappointed.

I'm not sure how to answer this. That seems like enumerating all the dependencies
of every dependency GDB has (bfd, libiberty, bison, etc). The feature (like the
C compile feature already included) requires the compiler plug-in. Whatever that
plug-in supports is what GDB supports.

Would you like me to include something like: "Currently tested on x86_64
GNU/Linux." That tells readers what is known to work without suggesting
all possible permutations of host/OS that might work [but are untested
AFAIK].

>> +set debug compile-oracle
>> +show debug compile-oracle
>> + Control the display of debug output about symbol requests from
>> + the compiler plug-in.
>> +
>> +set debug compile-cplus-types
>> +show debug compile-cplus-types
>> + Control the display of debug output about C++ type conversion
>> + in the compile feature.
>
> I suggest to mention that these commands are only available/have
> effect if the C++ compilation is supported.

Maybe this is better/clearer?

set debug compile-cplus-types
show debug compile-cplus-types
Control the display of debug output about type conversion in the
C++ compile feature. Commands have no effect while compiling for
other languages.

>> +@anchor{set debug compile-oracle}
>> +@item set debug compile-oracle
>> +@cindex compile oracle debugging info
>> +Turns on or off display of symbol requests from the compiler plug-in
>> +(the ``oracle''). The default is off.
>
> @dfn{oracle} should render better. In any case, why do we call this
> "the oracle"? In the computing context, "oracle" has connotations
> that we don't necessarily want to encourage, I think. Why not
> something like "set debug symbol-requests"?

This symbol resolver design is called the "oracle" internally (see also
references to this name in the Wiki page describing the compile project,
https://sourceware.org/gdb/wiki/GCCCompileAndExecute ).

Nonetheless, on second thought, it seems almost nonsensical to have a
special debug flag just for this. So I've removed the option and just
added the "oracle" debug output when the "normal" compile debug flag
is set. WDYT?

>> +Turns on or off the display of C++ type conversion debugging information.
>
> Please use C@t{++} for C++ in the text, it renders better in the
> printed manual.

Done.

Would you like me to repost this patch?

Keith