This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: pr 11067 patch
Hi Chris,
On Fri, 12 Feb 2010 16:48:13 +0100, Chris Moller wrote:
> --- gdb/testsuite/gdb.mi/mi-var-display.exp 1 Jan 2010 07:32:03 -0000 1.34
> +++ gdb/testsuite/gdb.mi/mi-var-display.exp 12 Feb 2010 15:38:07 -0000
> mi_gdb_test "-var-evaluate-expression anone" \
> - "\\^done,value=\"A\"" \
> + "\\^done,value=\"A = \\(enum <anonymous>\\)0\"" \
> "eval variable anone"
While it works with Eclipse should this format be used even for MI?
MI doc says:
returns its value as a string.
It is true it may be the same what `print' doc says:
By default, GDB prints a value according to its data type.
Still IMO the representation "A = (enum <anonymous>)0" is not suitable for MI
as it is not the intended type of a C expression.
But in practice for Eclipse it works and it brings better user experience - as
in CLI.
Some GNU Coding Standard nitpicks:
> Index: gdb/ChangeLog$
> +Wed Feb 10 17:13:44 2010 Chris Moller <moller@mollerware.com>
timestamp line does not follow GCS.
> + /*
> + When printing a simple enum value, the following code includes, in
> + addition to the symbolic value, the numeric value and the enum tag.
> + Under other-than-simple circumstances--in structs, arrays, etc.--
> + it does what's always done and prints just the symbolic value.
> + */
GCS comment should be formatted:
/* text...
text. */
> + else if (TYPE_TAG_NAME(type))
> + enum_name = TYPE_TAG_NAME(type);
GCS spacing: -> "TYPE_TAG_NAME (type)"
> + else
> + enum_name = "<anonymous>";
> +
= +^I $
git diff --check says:
gdb/c-valprint.c:433: trailing whitespace.
(but unaware how to check an already axisting .patch file)
> + fprintf_filtered (stream, "%s = (enum %s)%s",
->
# + fprintf_filtered (stream, "%s = (enum %s) %s",
while PR 11067 shows it that way GCS 5.3 suggests a whitespace there:
foo = (char *) malloc (sizeof *foo);
> +++ gdb/testsuite/gdb.base/pr11067.c 12 Feb 2010 15:38:05 -0000
> @@ -0,0 +1,48 @@
> +enum E {
> + Val1 = 56,
> + Val2
> +};
According to GNU indent it should be:
enum E
{
Val1 = 56,
Val2
};
> +struct Es {
ditto
> +int main() {
GCS:
int
main ()
{
> + /*
> + The following is a fake-out for linkers (such as, apparently, the AIX
> + linker) that optimises out unreference variables.
> + */
Comment -> GCS.
> + return 0;
> +}
> +
trailing whitespace line.
> +++ gdb/testsuite/gdb.base/pr11067.exp 12 Feb 2010 15:38:05 -0000
> +gdb_test "print e" "Val1 = \\(enum E\\)56"
> +
> +gdb_test "print ea" "{Val1, Val2, Val1}"
> +
Two spaces.
GDB testscases use more a practice to expect also leading " = ":
gdb_test "print e" " = Val1 = \\(enum E\\)56"
gdb_test "print ea" " = {Val1, Val2, Val1}"
> +gdb_test "print es" "v = 5, e = Val2}"
+ IMO missing opening bracket.
gdb_test "print es" " = {v = 5, e = Val2}"
Regards,
Jan