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

["Tom de Vries (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>]

Tom de Vries has posted comments on this change.

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


Patch Set 2:

(3 comments)

Thanks for the review, will upload a new version.

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

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/2/gdb/testsuite/lib/ada.exp@18 
PS2, Line 18: 
13 | # You should have received a copy of the GNU General Public License
14 | # along with this program.  If not, see <http://www.gnu.org/licenses/>.
15 | 
16 | 
17 | # Compile the ada code in $code to a file based on $name, using the flags
18 > # $compile_flag as well as debug, nowarning and quiet.
19 | # Return 1 if code can be compiled
20 | # Leave the file name of the resulting object in the upvar object.
21 | # Similar to gdb_simple_compile, but using gdb_compile_ada_1 instead of
22 | # gdb_compile.
23 | 

> `$compile_flags`

Done


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/2/gdb/testsuite/lib/ada.exp@60 
PS2, Line 60: 
55 | }
56 | 
57 | # Global variable used in gdb_ada_needs_libs_pic_suffix to detect recursion.
58 | # Initialize to 0.
59 | 
60 > global calculating_ada_needs_libs_pic_suffix
61 | set calculating_ada_needs_libs_pic_suffix 0
62 | 
63 | # Caching true/false for whether we need to use -lgnarl_pic -lgnat_pic in
64 | # target_compile_ada_from_dir.
65 | 

> Is this global statement needed, since we're already in the global scope?

Indeed, it's not really needed, removed.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/2/gdb/testsuite/lib/ada.exp@64 
PS2, Line 64: 
59 | 
60 | global calculating_ada_needs_libs_pic_suffix
61 | set calculating_ada_needs_libs_pic_suffix 0
62 | 
63 | # Caching true/false for whether we need to use -lgnarl_pic -lgnat_pic in
64 > # target_compile_ada_from_dir.
65 | 
66 | gdb_caching_proc gdb_ada_needs_libs_pic_suffix {
67 |     global calculating_ada_needs_libs_pic_suffix
68 |     if { $calculating_ada_needs_libs_pic_suffix } {
69 | 	return 0

> Can you explain shortly here the intent of the method used to detect whether those flags are needed? […]

Thanks, that's a nice addition.

I've fixed this by using this text to document the three return statements in the body.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I3e1e40bd46236b45e2d7808c1cd744a075d4a148
Gerrit-Change-Number: 32
Gerrit-PatchSet: 2
Gerrit-Owner: Tom de Vries <tdevries@suse.de>
Gerrit-Reviewer: Joel Brobecker <brobecker@adacore.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-CC: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Wed, 30 Oct 2019 17:35:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment