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] add simple attribute introspection


Ping: https://gcc.gnu.org/ml/gcc-patches/2018-10/msg01473.html

With the C++ bits approved I'm still looking for a review/approval
of the remaining changes: the C front end and the shared c-family
handling of the new built-in.

On 10/23/2018 04:08 PM, Martin Sebor wrote:
On 10/22/2018 04:08 PM, Jason Merrill wrote:
On 10/13/18 8:19 PM, Martin Sebor wrote:
+  oper = cp_parser_type_id (parser);
+  parser->in_type_id_in_expr_p = saved_in_type_id_in_expr_p;
+
+  if (cp_parser_parse_definitely (parser))
+    {
+      /* If all went well, set OPER to the type.  */
+      cp_decl_specifier_seq decl_specs;
+
+      /* Build a trivial decl-specifier-seq.  */
+      clear_decl_specs (&decl_specs);
+      decl_specs.type = oper;
+
+      /* Call grokdeclarator to figure out what type this is.  */
+      oper = grokdeclarator (NULL,
+                 &decl_specs,
+                 TYPENAME,
+                 /*initialized=*/0,
+                 /*attrlist=*/NULL);
+    }

Doesn't grokdeclarator here give you back the same type you already had
from cp_parser_type_id?  The latter already calls grokdeclarator.

I don't know why cp_parser_sizeof_operand does this, either.  Try
removing it from both places?

You're right, the call in cp_parser_has_attribute_expression
was unnecessary.  cp_parser_sizeof_operand still needs it.


+  /* Consume the comma if it's there.  */
+  if (!cp_parser_require (parser, CPP_COMMA, RT_COMMA))
+    {
+      parens.require_close (parser);

I think you want cp_parser_skip_to_closing_parenthesis for error
recovery, rather than require_close.

Thanks, the error messages look slightly better that way (there
are fewer of them), although still not as good as in C or other
compilers in some cases.


+  if (tree attr = cp_parser_gnu_attribute_list (parser,
/*exactly_one=*/true))
+    {
+      if (oper != error_mark_node)
+    {
+      /* Fold constant expressions used in attributes first.  */
+      cp_check_const_attributes (attr);
+
+      /* Finally, see if OPER has been declared with ATTR.  */
+      ret = has_attribute (atloc, oper, attr, default_conversion);
+    }
+    }
+  else
+    {
+      error_at (atloc, "expected identifier");
+      cp_parser_skip_to_closing_parenthesis (parser, true, false,
false);
+    }
+
+  parens.require_close (parser);

I think the require_close should be in the valid case, since *skip*
already consumes a closing paren.

Ah, I need to make it consume the paren by passing true as the last
argument.  With that it works.


+is valuated.  The @var{type-or-expression} argument is subject to the
same

evaluated

Thanks for the review.

Attached is an updated patch with the fixes above.

Martin



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