Re: [patch] [python] mi/12531

[Tom Tromey <tromey@redhat.com>]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> This patch fixes a case where an MI user could install a visualizer on a
Phil> synthetic varobj.  If we detect that the object is a CPLUS_FAKE_CHILD,
Phil> we do not install a visualizer.  I thought about raising an error, but
Phil> other MI behavior with fake children is just to move on if possible.  I
Phil> did the same.

Thanks for doing this.

I have some comments, but they are all minor things.

Phil> 2011-04-13  Phil Muldoon  <pmuldoon@redhat.com>
Phil> 	* varobj.c (install_default_visualizer): Do not install a
Phil> 	visualizer if the varobj is CPLUS_FAKE_CHILD.
Phil> 	(construct_visualizer): Likewise.

I thought there was a PR open for this.  If so, it should be mentioned
in the ChangeLog.  (If it is just in Red Hat bugzilla then it is
optional; but if you choose to use it, put the whole URL in.)

Phil> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++ additional_flags=-DMI}] != "" } {

This should be split somewhere.

Phil> +	untested "Couldn't compile ${srcfile} in c++ mode"
Phil> +	return -1

Wrong indentation, I think.

Phil> +    }

Definitely wrong indentation :)

Phil> +mi_continue_to_line [gdb_get_line_number {break to inspect struct and union} ${testfile}.c] \

Split this line somewhere.

Phil>  #ifdef __cplusplus
Phil> +#include <string>

It is best not to rely on libstdc++ in the test suite.

I think just an ordinary struct ought to exhibit the behavior you need,
as long as the file is compiled by the C++ compiler.  So, you probably
don't need any additional classes.  But, if you do, you have to write
them yourself :)

Phil>  static void
Phil>  install_default_visualizer (struct varobj *var)
Phil>  {
Phil> +
Phil> +  /* Do not install a visualizer on a CPLUS_FAKE_CHILD.  */
Phil> +  if (CPLUS_FAKE_CHILD (var))
Phil> +    return;

Spurious blank line after the "{".

Tom