[review] infcall, c++: collect more pass-by-reference information

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

Tom Tromey has posted comments on this change.

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


Patch Set 1: Code-Review+1

(3 comments)

Thank you.  I appreciate what you've done here (in particular the comment
on the test case, but really all of it looks quite good).
There were some nits in this one so I am marking it +1.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/137/1/gdb/gnu-v3-abi.c 
File gdb/gnu-v3-abi.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/137/1/gdb/gnu-v3-abi.c@1321 
PS1, Line 1321: 
1296 | is_copy_or_move_constructor_type (struct type *class_type,
     | ...
1316 |      then this is not a copy or move constructor, but just a
1317 |      constructor.  */
1318 |   for (int i = 2; i < TYPE_NFIELDS (method_type); i++)
1319 |     {
1320 |       arg_type = TYPE_FIELD_TYPE (method_type, i);
1321 |       /* FIXME taktemur/2019-04-23: As of this date, neither
1322 | 	 clang++-7.0.0 nor g++-8.2.0 produce a DW_AT_default_value
1323 | 	 attribute.  GDB is also not set to read this attribute, yet.
1324 | 	 Hence, we immediately return false if there are more than
1325 | 	 2 parameters.  */

It would be good to ensure that there is a gcc bug report
for this, and then link to the bug from this comment.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/137/1/gdb/gnu-v3-abi.c@1413 
PS1, Line 1413: 
1371 | gnuv3_pass_by_reference (struct type *type)
     | ...
1404 |       has_cc_attr = true;
1405 |       is_pass_by_value = false;
1406 |     }
1407 | 
1408 |   /* A dynamic class has a non-trivial copy constructor.
1409 |      See c++98 section 12.8 Copying class objects [class.copy].  */
1410 |   if (gnuv3_dynamic_class (type))
1411 |     is_dynamic = true;
1412 | 
1413 |   /* FIXME taktemur/2019-04-23: What if there are multiple copy ctors?

I think it would be good to answer this before landing.
Maybe we need to do overload resolution?


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/137/1/gdb/gnu-v3-abi.c@1495 
PS1, Line 1495: 
1371 | gnuv3_pass_by_reference (struct type *type)
     | ...
1486 |      about recursive loops here, since we are only looking at members
1487 |      of complete class type.  Also ignore any static members.  */
1488 |   for (fieldnum = 0; fieldnum < TYPE_NFIELDS (type); fieldnum++)
1489 |     if (!field_is_static (&TYPE_FIELD (type, fieldnum)))
1490 |       {
1491 | 	struct type *field_type = TYPE_FIELD_TYPE (type, fieldnum);
1492 | 
1493 | 	/* For arrays, make the decision based on the element type.  */
1494 | 	if (TYPE_CODE (field_type) == TYPE_CODE_ARRAY)
1495 | 	  field_type = field_type->main_type->target_type;

I think it's more usual to use the TYPE_TARGET_TYPE
accessor here.  Also normally one must call check_typedef
on the result.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ic05bd98a962d07ec3c1ad041f709687eabda3bb9
Gerrit-Change-Number: 137
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 29 Oct 2019 22:13:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment