This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] v4: C++: more location wrapper nodes (PR c++/43064, PR c++/43486)


On 12/13/18 3:12 PM, David Malcolm wrote:
On Wed, 2018-12-12 at 15:37 -0500, Jason Merrill wrote:
On 12/7/18 3:13 PM, David Malcolm wrote:
On Tue, 2018-12-04 at 18:31 -0500, Jason Merrill wrote:
On 12/3/18 5:10 PM, Jeff Law wrote:
On 11/19/18 9:51 AM, David Malcolm wrote:

[...]
@@ -1058,6 +1058,9 @@ grokbitfield (const cp_declarator
*declarator,
         return NULL_TREE;
       }
+ if (width)
+    STRIP_ANY_LOCATION_WRAPPER (width);

Why is this needed?  We should already be reducing width to an
unwrapped
constant value somewhere along the line.

"width" is coming from cp_parser_member_declaration, from:

	      /* Get the width of the bitfield.  */
	      width = cp_parser_constant_expression (parser, false,
NULL,
						     cxx_dialect >=
cxx11);

and currently nothing is unwrapping the value.  We presumably need
to
unwrap (or fold?) it before it is stashed in
    DECL_BIT_FIELD_REPRESENTATIVE (value).

Without stripping (or folding) here, we e.g. lose a warning and get
this:
    FAIL: g++.dg/abi/empty22.C  -std=gnu++98  (test for warnings,
line 15)

Why does that happen?  check_bitfield_decl ought to handle the
location
wrapper fine.  That's where it gets folded.

The unstripped location wrapper defeats this check for zero in
check_field_decls within cp/class.c:

3555	      if (DECL_C_BIT_FIELD (x)
3556		  && integer_zerop (DECL_BIT_FIELD_REPRESENTATIVE (x)))
3556		/* We don't treat zero-width bitfields as making a class
3557		   non-empty.  */

Aha. I wonder if integer_zerop should look through location wrappers? Or alternately, abort if it gets one?

On a tangent, perhaps we also want a macro like TREE_CODE that looks through wrappers. TREE_CODE_WRAPPED? _NO_WRAP? Other name ideas?

3558		;
3559	      else

leading it to erroneously use the "else" clause, which thus erroneously
clears CLASSTYPE_EMPTY_P, leading to the loss of:

g++.dg/abi/empty22.C:15:6: warning: empty class 'dummy' parameter passing
   ABI changes in -fabi-version=12 (GCC 8) [-Wabi]
    15 |   fun(d, f); // { dg-warning "empty" }
       |   ~~~^~~~~~

check_bitfield_decl is called *after* that check, so the folding there
doesn't help.

@@ -656,6 +656,9 @@ add_capture (tree lambda, tree id, tree
orig_init, bool by_reference_p,
         listmem = make_pack_expansion (member);
         initializer = orig_init;
       }
+
+  STRIP_ANY_LOCATION_WRAPPER (initializer);

Why is this needed?  What cares about the tree codes of the
capture
initializer?

This is used to populate LAMBDA_EXPR_CAPTURE_LIST.  Without
stripping,
we end up with wrapped VAR_DECLs, rather than the VAR_DECLs
themselves,

Sure, that sounds fine.

and this confuses things later on, for example leading to:

   PASS -> FAIL : g++.dg/cpp0x/lambda/lambda-type.C  -std=c++14
(test for excess errors)
   PASS -> FAIL : g++.dg/cpp0x/lambda/lambda-type.C  -std=c++17
(test for excess errors)

Confuses how?

Without stripping, we get extra errors for decltype() on identifiers in
the capture-list, e.g.:

   [i] {
     same_type<decltype(i),int>();
     same_type<decltype((i)),int const&>();
   };

cpp0x/lambda/lambda-type.C: In lambda function:
cpp0x/lambda/lambda-type.C:48:27: error: 'i' is not captured
    48 |     same_type<decltype((i)),int const&>();
       |                           ^
cpp0x/lambda/lambda-type.C:48:39: error: template argument 1 is invalid
    48 |     same_type<decltype((i)),int const&>();
       |                                       ^

These occur because, in capture_decltype, when searching for the decl for
"i" here:

   tree cap = value_member (decl, LAMBDA_EXPR_CAPTURE_LIST (lam));

the LAMBDA_EXPR_CAPTURE_LIST contains a wrapper around "i" rather than "i"
itself, and so "i" isn't found by value_member; "cap" thus becomes NULL_TREE,
and thus the error.

Ah.  Just above that I notice,

  /* FIXME do lookup instead of list walk? */

We could make that change, i.e.

tree cap = lookup_name_real (DECL_NAME (decl), /*type*/0, /*nonclass*/1,
                             /*block_p=*/true, /*ns*/0, LOOKUP_HIDDEN);
...
if (cap && is_capture_proxy (cap))

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]