Re: [RFA] Allow setting breakpoints on inline functions (PR 10738)

[Gary Benson <gbenson@redhat.com>]

Jan Kratochvil wrote:
> On Tue, 29 Nov 2011 16:02:00 +0100, Gary Benson wrote:
> > I'm also interested in feedback about the tests I've written,
> > since this is the first time I've worked on the testsuite.
> 
> I would prefer definitely a copy in gdb.dwarf2/ .  gdb.opt/ I find a
> bad idea as it breaks too much across GCC changes.  just gcc -S -dA
> is probably OK.
> 
> If there is gdb.dwarf2/ then I would even drop the gdb.opt/ one.
> Not sure if it makes sense for non-DWARF targets, the functionality
> gets tested by the DWARF targets anyway.

Are you saying I should move the testcase from gdb.opt and into
gdb.dwarf2?  I can do that.

Is there some difference between the way the various directories
of tests are treated?  And, is there something I should read to help
decide where to put tests?

> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.opt/inline-break.exp
> ...
> > +if { [prepare_for_testing $testfile.exp $testfile $testfile.c \
> > +          {debug optimize=-O2 additional_flags=-Winline}] } {
> 
> This -Winline is questionable.
> 
> You make the .c file compatible even if __GNUC__ is not defined but
> then you use GCC-specific -Winline option.  Either just make the
> testcase whole GCC-specific or make a fallback if the compilation
> with -Winline fails try also non-Winline.  Or maybe just drop that
> -Winline, there is no -Werror anyway so it was more just for the
> testcase development.

As I understand it the -Winline is there to cause the test to fail
if the methods don't get inlined.  I may very well be wrong, I don't
understand the syntax 100%, but as I read it any unexpected compiler
output causes gdb_compile to assume the compilation failed.

I copied this (somewhat) from the other gdb.opt/inline* testcases,
so if there is a problem with it then I guess they should be fixed
too.

> As I suggested the gdb.dwarf2/ way this whole -Winline is offtopic
> then.
> 
> > +#
> > +# func1 is a static inlined function that is called once.
> > +# The result should be a single-location breakpoint.
> > +#
> > +gdb_test "break func1" \
> > +    "Breakpoint.*at.* file .*$testfile\.c, line.*"
> 
> As you use "" and not {} you should use \\.c and not \.c .  This way
> it is the same as .c .
> 
> It is there several times.
> 
> > +# func2 is a non-static inlined function that is called once.
> > +# The result should be a breakpoint with two locations: the
> > +# out-of-line function and the single inlined instance.
> > +#
> > +gdb_test "break func2" \
> > +    "Breakpoint.*at.*func2.*(2 locations).*"
> 
> You wanted to use \\( and \\) here.
> 
> It is there several times.

Thanks, I added the extra '\'s on my branch.  Would it be better
to to use {} here, or does that make other changes?

Cheers,
Gary

-- 
http://gbenson.net/