[review] [gdb/testsuite] Compile ada with -lgnarl_pic and -lgnat_pic if required

Luis Machado (Code Review)
2019-10-21T20:52:34Z

Luis Machado has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32
......................................................................

Patch Set 1:

(4 comments)

At first the change looked a bit too complicated for what it does. But i realized that, if we don't want to keep compiling things incorrectly over and over again, caching the setting is the only way to do it. And that makes the code a bit larger.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp
File gdb/testsuite/lib/ada.exp:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp@16
PS1, Line 16: # Call target_compile with SOURCE DEST TYPE and OPTIONS as argument,
: # after having temporarily changed the current working directory to
: # BUILDDIR.
The location of this comment needs to be adjusted, since it is now out of place.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp@20
PS1, Line 20: proc gdb_simple_compile_ada {name code {type object} {compile_flags {}} {object obj}} {
Some documentation for this function would be nice. It seems to be a helper function that returns true/false for whether a compilation succeeded or not?

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp@55
PS1, Line 55: gdb_caching_proc gdb_ada_needs_libs_pic_suffix {
It would be nice to have some documentation here explaining that we're caching true/false for whether we need to use the special workaround compilation flags.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp@131
PS1, Line 131: # Compile some Ada code.
I think this is now a helper function? Should we adjust its documentation a bit?

My understanding is that gdb_compile_ada_1 now only silently tests a compilation and doesn't actually gdb_compile_test it?