(no subject)

[Iain Sandoe]

This patch set is not actually particulalry new, I have been maintaining
it locally one Darwin branches and it has been tested on several versions
of Darwin both with and without Alex's __has_{feature, extension} patch.

This is one of the three most significant blockers to importing the macOS
SDKs properly, and cannot currently be fixincludes-ed (in fact it can not
ever really since the attribute is uaer-facing and so can be in end-user
code that we cannot fix).

OK for trunk?
thanks
Iain

[PATCH 1/4] c-family: Add handling for clang-style attributes [PR109877].

[Iain Sandoe]

This patch set is not actually particualry new, I have been maintaining
it locally one Darwin branches and it has been tested on several versions
of Darwin both with and without Alex's __has_{feature, extension} patch.

This is one of the three most significant blockers to importing the macOS
SDKs properly, and cannot currently be fixincludes-ed (in fact it can not
ever really since the attribute is uaer-facing and so can be in end-user
code that we cannot fix).

OK for trunk?
thanks
Iain

--- 8< ---


The clang compiler supports essentially arbitrary, per-attribute, syntax and
token forms for attribute arguments.  This extends to the case where token
forms are required to be accepted that are not part of the valid set for
standard C or C++.

A motivating  example (in the initial attribute of this form implemented
in this patch set) is version-style (i.e. x.y.z) numeric values.  At present
the c-family cannot handle this, since invalid numeric tokens are rejected
by both C and C++ frontends before we have a chance to decide to accept them
in custom attribute argument parsing.

The solution proposed in this patch series is to allow for a certain set of
attributes names that are known to be 'clang-form' and to defer argument
token validation until the parse of those arguments.

This does not apparently represent any loss of generality - since the
specific attribute names are already claimed by clang and re-using them with
different semantics in GCC would be a highly unfortunate experience for end-
users.

The first patch here adds a mechanism to check attribute identifiers against
a list known to be in clang form.  The 'availability' attribute is added as a
first example.

The acceptance of non-standard tokens is constrained to the interval enclosing
the attribute arguments of cases notified as 'clang-form'.

	PR c++/109877

gcc/c-family/ChangeLog:

	* c-attribs.cc (attribute_clang_form_p): New.
	* c-common.h (attribute_clang_form_p): New.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
---
 gcc/c-family/c-attribs.cc | 12 ++++++++++++
 gcc/c-family/c-common.h   |  1 +
 2 files changed, 13 insertions(+)

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 461732f60f7..8c087317f4f 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -615,6 +615,18 @@ attribute_takes_identifier_p (const_tree attr_id)
     return targetm.attribute_takes_identifier_p (attr_id);
 }
 
+/* Returns TRUE iff the attribute indicated by ATTR_ID needs its
+   arguments converted to string constants.  */
+
+bool
+attribute_clang_form_p (const_tree attr_id)
+{
+  const struct attribute_spec *spec = lookup_attribute_spec (attr_id);
+  if (spec && !strcmp ("availability", spec->name))
+    return true;
+  return false;
+}
+
 /* Verify that argument value POS at position ARGNO to attribute NAME
    applied to function FN (which is either a function declaration or function
    type) refers to a function parameter at position POS and the expected type
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index b57e83d7c5d..4dbc566d2b5 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1535,6 +1535,7 @@ extern void check_for_xor_used_as_pow (location_t lhs_loc, tree lhs_val,
 /* In c-attribs.cc.  */
 extern bool attribute_takes_identifier_p (const_tree);
 extern tree handle_deprecated_attribute (tree *, tree, tree, int, bool *);
+extern bool attribute_clang_form_p (const_tree);
 extern tree handle_unused_attribute (tree *, tree, tree, int, bool *);
 extern tree handle_fallthrough_attribute (tree *, tree, tree, int, bool *);
 extern int parse_tm_stmt_attr (tree, int);
-- 
2.39.2 (Apple Git-143)

[PATCH 2/4] c-family, C: handle clang attributes [PR109877].

[Iain Sandoe]

This adds the ability to defer the validation of numeric attribute
arguments until the sequence is parsed if the attribute being
handled is one known to be 'clang form'.

We do this by considering the arguments to be strings regardless
of content and defer the interpretation of those strings until the
argument processing.

	PR c++/109877

gcc/c-family/ChangeLog:

	* c-lex.cc (c_lex_with_flags): Allow for the case where
	we wish to defer interpretation of numeric values until
	parse time.
	* c-pragma.h (C_LEX_NUMBER_AS_STRING): New.

gcc/c/ChangeLog:

	* c-parser.cc (struct c_parser): Provide a flag to notify
        that argument parsing should return attribute arguments
        as string constants.
	(c_lex_one_token): Act to defer numeric value validation.
	(c_parser_clang_attribute_arguments): New.
	(c_parser_gnu_attribute): Allow for clang-form GNU-style
	attributes.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
---
 gcc/c-family/c-lex.cc   |  15 ++++++
 gcc/c-family/c-pragma.h |   3 ++
 gcc/c/c-parser.cc       | 109 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 122 insertions(+), 5 deletions(-)

diff --git a/gcc/c-family/c-lex.cc b/gcc/c-family/c-lex.cc
index 06c2453c89a..d535f5b460c 100644
--- a/gcc/c-family/c-lex.cc
+++ b/gcc/c-family/c-lex.cc
@@ -533,6 +533,21 @@ c_lex_with_flags (tree *value, location_t *loc, unsigned char *cpp_flags,
 
     case CPP_NUMBER:
       {
+	/* If the user wants number-like entities to be returned as a raw
+	   string, then don't try to classify them, which emits unwanted
+	   diagnostics.  */
+	if (lex_flags & C_LEX_NUMBER_AS_STRING)
+	  {
+	    /* build_string adds a trailing NUL at [len].  */
+	    tree num_string = build_string (tok->val.str.len + 1,
+					    (const char *) tok->val.str.text);
+	    TREE_TYPE (num_string) = char_array_type_node;
+	    *value = num_string;
+	    /* We will effectively note this as CPP_N_INVALID, because we
+	       made no checks here.  */
+	    break;
+	  }
+
 	const char *suffix = NULL;
 	unsigned int flags = cpp_classify_number (parse_in, tok, &suffix, *loc);
 
diff --git a/gcc/c-family/c-pragma.h b/gcc/c-family/c-pragma.h
index 98177913053..11cde74f9f0 100644
--- a/gcc/c-family/c-pragma.h
+++ b/gcc/c-family/c-pragma.h
@@ -276,6 +276,9 @@ extern void pragma_lex_discard_to_eol ();
 #define C_LEX_STRING_NO_JOIN	  2 /* Do not concatenate strings
 				       nor translate them into execution
 				       character set.  */
+#define C_LEX_NUMBER_AS_STRING	  4 /* Do not classify a number, but
+				       instead return it as a raw
+				       string.  */
 
 /* This is not actually available to pragma parsers.  It's merely a
    convenient location to declare this function for c-lex, after
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 703f9570dbc..aaaa16cc05d 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -217,6 +217,9 @@ struct GTY(()) c_parser {
      should translate them to the execution character set (false
      inside attributes).  */
   BOOL_BITFIELD translate_strings_p : 1;
+  /* True if we want to lex arbitrary number-like sequences as their
+     string representation.  */
+  BOOL_BITFIELD lex_number_as_string : 1;
 
   /* Objective-C specific parser/lexer information.  */
 
@@ -308,10 +311,10 @@ c_lex_one_token (c_parser *parser, c_token *token, bool raw = false)
 
   if (raw || vec_safe_length (parser->raw_tokens) == 0)
     {
+      int lex_flags = parser->lex_joined_string ? 0 : C_LEX_STRING_NO_JOIN;
+      lex_flags |= parser->lex_number_as_string ? C_LEX_NUMBER_AS_STRING : 0;
       token->type = c_lex_with_flags (&token->value, &token->location,
-				      &token->flags,
-				      (parser->lex_joined_string
-				       ? 0 : C_LEX_STRING_NO_JOIN));
+				      &token->flags, lex_flags);
       token->id_kind = C_ID_NONE;
       token->keyword = RID_MAX;
       token->pragma_kind = PRAGMA_NONE;
@@ -5210,6 +5213,98 @@ c_parser_gnu_attribute_any_word (c_parser *parser)
   return attr_name;
 }
 
+/* Handle parsing clang-form attribute arguments, where we need to adjust
+   the parsing rules to relate to a specific attribute.  */
+
+static tree
+c_parser_clang_attribute_arguments (c_parser *parser, tree /*attr_id*/)
+{
+  /* We can, if required, alter the parsing on the basis of the attribute.
+     At present, we handle the availability attr, where ach entry can be :
+	identifier
+	identifier=N.MM.Z
+	identifier="string"
+	followed by ',' or ) for the last entry*/
+
+  tree attr_args = NULL_TREE;
+  if (c_parser_next_token_is (parser, CPP_NAME)
+      && c_parser_peek_token (parser)->id_kind == C_ID_ID
+      && c_parser_peek_2nd_token (parser)->type == CPP_COMMA)
+    {
+      tree platf = c_parser_peek_token (parser)->value;
+      c_parser_consume_token (parser);
+      attr_args = tree_cons (NULL_TREE, platf, NULL_TREE);
+    }
+  else
+    {
+      c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
+				"expected a platform name followed by %<,%>");
+      return error_mark_node;
+    }
+  c_parser_consume_token (parser); /* consume the ',' */
+  do
+    {
+      tree name = NULL_TREE;
+      tree value = NULL_TREE;
+
+      if (c_parser_next_token_is (parser, CPP_NAME)
+	  && c_parser_peek_token (parser)->id_kind == C_ID_ID)
+	{
+	  name = c_parser_peek_token (parser)->value;
+	  c_parser_consume_token (parser);
+	}
+      else
+	{
+	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
+				     "expected an attribute keyword");
+	  return error_mark_node;
+	}
+      if (c_parser_next_token_is (parser, CPP_EQ))
+	{
+	  c_parser_consume_token (parser); /* eat the '=' */
+	  /* We need to bludgeon the lexer into not trying to interpret the
+	     xx.yy.zz form, since that just looks like a malformed float.
+	     Also, as a result of macro processing, we can have strig literals
+	     that are in multiple pieces so, for this specific part of the
+	     parse, we need to join strings.  */
+	  bool saved_join_state = parser->lex_joined_string;
+	  parser->lex_number_as_string = 1;
+	  parser->lex_joined_string = 1;
+	  /* So look at the next token, expecting a string, or something that
+	     looks initially like a number, but might be a version number.  */
+	  c_parser_peek_token (parser);
+	  /* Done with the funky number parsing.  */
+	  parser->lex_number_as_string = 0;
+	  parser->lex_joined_string = saved_join_state;
+	  if (c_parser_next_token_is_not (parser, CPP_CLOSE_PAREN)
+	      && c_parser_next_token_is_not (parser, CPP_COMMA))
+	    {
+	      value = c_parser_peek_token (parser)->value;
+	      /* ???: check for error mark and early-return?  */
+	      c_parser_consume_token (parser);
+	    }
+	  else
+	    {
+	      c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
+					 "expected a value");
+	      return error_mark_node;
+	    }
+	}
+      else if (c_parser_next_token_is_not (parser, CPP_CLOSE_PAREN)
+	       && c_parser_next_token_is_not (parser, CPP_COMMA))
+	{
+	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
+				     "expected %<,%> or %<=%>");
+	  return error_mark_node;
+	}
+    if (c_parser_next_token_is (parser, CPP_COMMA))
+      c_parser_consume_token (parser); /* Just skip the comma.  */
+    tree t = tree_cons (value, name, NULL);
+    chainon (attr_args, t);
+  } while (c_parser_next_token_is_not (parser, CPP_CLOSE_PAREN));
+  return attr_args;
+}
+
 /* Parse attribute arguments.  This is a common form of syntax
    covering all currently valid GNU and standard attributes.
 
@@ -5375,9 +5470,13 @@ c_parser_gnu_attribute (c_parser *parser, tree attrs,
       attrs = chainon (attrs, attr);
       return attrs;
     }
-  c_parser_consume_token (parser);
+  c_parser_consume_token (parser); /* The '('.  */
 
-  tree attr_args
+  tree attr_args;
+  if (attribute_clang_form_p (attr_name))
+    attr_args = c_parser_clang_attribute_arguments (parser, attr_name);
+  else
+    attr_args
     = c_parser_attribute_arguments (parser,
 				    attribute_takes_identifier_p (attr_name),
 				    false,
-- 
2.39.2 (Apple Git-143)

[PATCH 3/4] c-family, C++: Handle clang attributes [PR109877].

[Iain Sandoe]

This adds the ability to defer the validation of numeric attribute
arguments until the sequence is parsed if the attribute being
handled is one known to be 'clang form'.

We do this by considering the arguments to be strings regardless
of content and defer the interpretation of those strings until the
argument processing.

Since the C++ front end lexes tokens separately (and would report
non-compliant numeric tokens before we can intercept them), we need
to implement a small state machine to track the lexing of attributes.

	PR c++/109877

gcc/cp/ChangeLog:

	* parser.cc (enum clang_attr_state): New.
	(cp_lexer_attribute_state): New.
	(cp_lexer_new_main): Set initial attribute lexing state.
	(cp_lexer_get_preprocessor_token): Handle lexing of clang-
	form attributes.
	(cp_parser_clang_attribute): Handle clang-form attributes.
	(cp_parser_gnu_attribute_list): Switch to clang-form parsing
	where needed.
	* parser.h : New flag to signal lexing clang-form attributes.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
---
 gcc/cp/parser.cc | 230 +++++++++++++++++++++++++++++++++++++++++++++--
 gcc/cp/parser.h  |   3 +
 2 files changed, 227 insertions(+), 6 deletions(-)

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 5116bcb78f6..c12f473f2e3 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -699,6 +699,91 @@ cp_lexer_handle_early_pragma (cp_lexer *lexer)
 static cp_parser *cp_parser_new (cp_lexer *);
 static GTY (()) cp_parser *the_parser;
 
+/* Context-sensitive parse-checking for clang-style attributes.  */
+
+enum clang_attr_state {
+  CA_NONE = 0,
+  CA_ATTR,
+  CA_BR1, CA_BR2,
+  CA_LIST,
+  CA_LIST_ARGS,
+  CA_IS_CA,
+  CA_CA_ARGS,
+  CA_LIST_CONT
+};
+
+/* State machine tracking context of attribute lexing.  */
+
+static enum clang_attr_state
+cp_lexer_attribute_state (cp_token& token, enum clang_attr_state attr_state)
+{
+  /* Implement a context-sensitive parser for clang attributes.
+     We detect __attribute__((clang_style_attribute (ARGS))) and lex the
+     args ARGS with the following differences from GNU attributes:
+	(a) number-like values are lexed as strings [this allows lexing XX.YY.ZZ
+	   version numbers].
+	(b) we concatenate strings, since clang attributes allow this too.  */
+  switch (attr_state)
+    {
+    case CA_NONE:
+      if (token.type == CPP_KEYWORD
+	  && token.keyword == RID_ATTRIBUTE)
+	attr_state = CA_ATTR;
+      break;
+    case CA_ATTR:
+      if (token.type == CPP_OPEN_PAREN)
+	attr_state = CA_BR1;
+      else
+	attr_state = CA_NONE;
+      break;
+    case CA_BR1:
+      if (token.type == CPP_OPEN_PAREN)
+	attr_state = CA_BR2;
+      else
+	attr_state = CA_NONE;
+      break;
+    case CA_BR2:
+      if (token.type == CPP_NAME)
+	{
+	  tree identifier = (token.type == CPP_KEYWORD)
+	    /* For keywords, use the canonical spelling, not the
+	       parsed identifier.  */
+	    ? ridpointers[(int) token.keyword]
+	    : token.u.value;
+	  identifier = canonicalize_attr_name (identifier);
+	  if (attribute_clang_form_p (identifier))
+	    attr_state = CA_IS_CA;
+	  else
+	    attr_state = CA_LIST;
+	}
+      else
+	attr_state = CA_NONE;
+      break;
+    case CA_IS_CA:
+    case CA_LIST:
+      if (token.type == CPP_COMMA)
+	attr_state = CA_BR2; /* Back to the list outer.  */
+      else if (token.type == CPP_OPEN_PAREN)
+	attr_state = attr_state == CA_IS_CA ? CA_CA_ARGS
+					    : CA_LIST_ARGS;
+      else
+	attr_state = CA_NONE;
+      break;
+    case CA_CA_ARGS: /* We will special-case args in this state.  */
+    case CA_LIST_ARGS:
+      if (token.type == CPP_CLOSE_PAREN)
+	attr_state = CA_LIST_CONT;
+      break;
+    case CA_LIST_CONT:
+      if (token.type == CPP_COMMA)
+	attr_state = CA_BR2; /* Back to the list outer.  */
+      else
+	attr_state = CA_NONE;
+      break;
+    }
+  return attr_state;
+}
+
 /* Create a new main C++ lexer, the lexer that gets tokens from the
    preprocessor, and also create the main parser.  */
 
@@ -715,6 +800,9 @@ cp_lexer_new_main (void)
   c_common_no_more_pch ();
 
   cp_lexer *lexer = cp_lexer_alloc ();
+  lexer->lex_number_as_string_p = false;
+  enum clang_attr_state attr_state = CA_NONE;
+
   /* Put the first token in the buffer.  */
   cp_token *tok = lexer->buffer->quick_push (token);
 
@@ -738,8 +826,20 @@ cp_lexer_new_main (void)
       if (tok->type == CPP_PRAGMA_EOL)
 	cp_lexer_handle_early_pragma (lexer);
 
+      attr_state = cp_lexer_attribute_state (*tok, attr_state);
       tok = vec_safe_push (lexer->buffer, cp_token ());
-      cp_lexer_get_preprocessor_token (C_LEX_STRING_NO_JOIN, tok);
+      unsigned int flags = C_LEX_STRING_NO_JOIN;
+      if (attr_state == CA_CA_ARGS)
+	{
+	  /* We are handling a clang attribute;
+	     1. do not try to diagnose x.y.z as a bad number, it could be a
+		version.
+	     2. join strings.  */
+          flags = C_LEX_NUMBER_AS_STRING;
+	  lexer->lex_number_as_string_p = true;
+	}
+      cp_lexer_get_preprocessor_token (flags, tok);
+      lexer->lex_number_as_string_p = false;
     }
 
   lexer->next_token = lexer->buffer->address ();
@@ -956,7 +1056,15 @@ cp_lexer_get_preprocessor_token (unsigned flags, cp_token *token)
 {
   static int is_extern_c = 0;
 
-   /* Get a new token from the preprocessor.  */
+   /* Get a new token from the preprocessor.
+  int lex_flags = 0;
+  if (lexer != NULL)
+    {
+      lex_flags = C_LEX_STRING_NO_JOIN;
+      if (lexer->lex_number_as_string_p)
+	lex_flags |= C_LEX_NUMBER_AS_STRING;
+    }
+  */
   token->type
     = c_lex_with_flags (&token->u.value, &token->location, &token->flags,
 			flags);
@@ -29302,6 +29410,113 @@ cp_parser_gnu_attributes_opt (cp_parser* parser)
   return attributes;
 }
 
+/* Parse the arguments list for a clang attribute.   */
+static tree
+cp_parser_clang_attribute (cp_parser *parser, tree/*attr_id*/)
+{
+  /* Each entry can be :
+     identifier
+     identifier=N.MM.Z
+     identifier="string"
+     followed by ',' or ) for the last entry*/
+
+  matching_parens parens;
+  if (!parens.require_open (parser))
+    return NULL;
+
+  bool save_translate_strings_p = parser->translate_strings_p;
+  parser->translate_strings_p = false;
+  tree attr_args = NULL_TREE;
+  cp_token *token = cp_lexer_peek_token (parser->lexer);
+  if (token->type == CPP_NAME
+      && cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_COMMA)
+    {
+      tree platf = token->u.value;
+      cp_lexer_consume_token (parser->lexer);
+      attr_args = tree_cons (NULL_TREE, platf, NULL_TREE);
+    }
+  else
+    {
+      error_at (token->location,
+		"expected a platform name followed by %<,%>");
+      cp_parser_skip_to_closing_parenthesis (parser,
+					     /*recovering=*/true,
+					     /*or_comma=*/false,
+					     /*consume_paren=*/true);
+      parser->translate_strings_p = save_translate_strings_p;
+      return error_mark_node;
+    }
+
+  cp_lexer_consume_token (parser->lexer); /* consume the ',' */
+  do
+    {
+      tree name = NULL_TREE;
+      tree value = NULL_TREE;
+
+      token = cp_lexer_peek_token (parser->lexer);
+      if (token->type == CPP_NAME)
+	{
+	  name = token->u.value;
+	  cp_lexer_consume_token (parser->lexer);
+	}
+      else
+	{
+	  /* FIXME: context-sensitive for that attrib.  */
+	  error_at (token->location, "expected an attribute keyword");
+	  cp_parser_skip_to_closing_parenthesis (parser,
+						 /*recovering=*/true,
+						 /*or_comma=*/false,
+						 /*consume_paren=*/true);
+	  attr_args = error_mark_node;
+	  break;
+	}
+
+      if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
+	{
+	  cp_lexer_consume_token (parser->lexer); /* eat the '=' */
+	  token = cp_lexer_peek_token (parser->lexer);
+	  if (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN)
+	      && cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
+	    {
+	      value = token->u.value;
+	      /* ???: check for error mark and early-return?  */
+	      cp_lexer_consume_token (parser->lexer);
+	    }
+	  else
+	    {
+	      error_at (token->location, "expected a value");
+	      cp_parser_skip_to_closing_parenthesis (parser,
+						     /*recovering=*/true,
+						     /*or_comma=*/false,
+						     /*consume_paren=*/true);
+	      attr_args = error_mark_node;
+	      break;
+	    }
+	}
+      else if (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN)
+	       && cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
+	{
+	  error_at (token->location, "expected %<,%>, %<=%> or %<)%>");
+	  cp_parser_skip_to_closing_parenthesis (parser,
+						 /*recovering=*/true,
+						 /*or_comma=*/false,
+						 /*consume_paren=*/true);
+	  attr_args = error_mark_node;
+	  break;
+	}
+    if (cp_lexer_next_token_is (parser->lexer, CPP_COMMA))
+      cp_lexer_consume_token (parser->lexer);
+    tree t = tree_cons (value, name, NULL_TREE);
+    attr_args = chainon (attr_args, t);
+  } while (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN));
+
+  parser->translate_strings_p = save_translate_strings_p;
+  if (!parens.require_close (parser))
+    return error_mark_node;
+
+  return attr_args;
+}
+
 /* Parse a GNU attribute-list.
 
    attribute-list:
@@ -29361,9 +29576,12 @@ cp_parser_gnu_attribute_list (cp_parser* parser, bool exactly_one /* = false */)
 
 	  /* Peek at the next token.  */
 	  token = cp_lexer_peek_token (parser->lexer);
-	  /* If it's an `(', then parse the attribute arguments.  */
-	  if (token->type == CPP_OPEN_PAREN)
+	  if (token->type == CPP_OPEN_PAREN
+	      && attribute_clang_form_p (identifier))
+	    arguments = cp_parser_clang_attribute (parser, identifier);
+	  else if (token->type == CPP_OPEN_PAREN)
 	    {
+	      /* If it's an `(', then parse the attribute arguments.  */
 	      vec<tree, va_gc> *vec;
 	      int attr_flag = (attribute_takes_identifier_p (identifier)
 			       ? id_attr : normal_attr);
@@ -29380,12 +29598,12 @@ cp_parser_gnu_attribute_list (cp_parser* parser, bool exactly_one /* = false */)
 		  arguments = build_tree_list_vec (vec);
 		  release_tree_vector (vec);
 		}
-	      /* Save the arguments away.  */
-	      TREE_VALUE (attribute) = arguments;
 	    }
 
 	  if (arguments != error_mark_node)
 	    {
+	      /* Save the arguments away.  */
+	      TREE_VALUE (attribute) = arguments;
 	      /* Add this attribute to the list.  */
 	      TREE_CHAIN (attribute) = attribute_list;
 	      attribute_list = attribute;
diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
index ee3afbaae04..e75c7abac5e 100644
--- a/gcc/cp/parser.h
+++ b/gcc/cp/parser.h
@@ -125,6 +125,9 @@ struct GTY (()) cp_lexer {
   /* True for in_omp_attribute_pragma lexer that should be destroyed
      when it is no longer in use.  */
   bool orphan_p;
+
+  /* TRUE if we want to parse number-like tokens as a string cst.  */
+  bool lex_number_as_string_p;
 };
 
 
-- 
2.39.2 (Apple Git-143)

[PATCH 4/4] Darwin: Implement clang availability attribute [PR109877].

[Iain Sandoe]

This implements the handling of the clang-form "availability"
attribute, which is the most important case used in the the macOS
SDKs.

	PR  c++/109877

gcc/ChangeLog:

	* config/darwin-protos.h
	(darwin_handle_weak_import_attribute): New.
	(darwin_handle_availability_attribute): New.
	(darwin_attribute_takes_identifier_p): New.
	* config/darwin.cc (objc_method_decl): New.
	(enum version_components): New.
	(parse_version): New.
	(version_from_version_array): New.
	(os_version_from_avail_value): New.
	(NUM_AV_OSES, NUM_AV_CLAUSES): New.
	(darwin_handle_availability_attribute): New.
	(darwin_attribute_takes_identifier_p): New.
	(darwin_override_options): New.
	* config/darwin.h (TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P): New.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
---
 gcc/config/darwin-protos.h |   9 +-
 gcc/config/darwin.cc       | 343 +++++++++++++++++++++++++++++++++++++
 gcc/config/darwin.h        |   7 +-
 3 files changed, 355 insertions(+), 4 deletions(-)

diff --git a/gcc/config/darwin-protos.h b/gcc/config/darwin-protos.h
index 9df358ee7d3..0702db25178 100644
--- a/gcc/config/darwin-protos.h
+++ b/gcc/config/darwin-protos.h
@@ -86,9 +86,12 @@ extern void darwin_asm_lto_end (void);
 extern void darwin_mark_decl_preserved (const char *);
 
 extern tree darwin_handle_kext_attribute (tree *, tree, tree, int, bool *);
-extern tree darwin_handle_weak_import_attribute (tree *node, tree name,
-						 tree args, int flags,
-						 bool * no_add_attrs);
+extern tree darwin_handle_weak_import_attribute (tree *, tree, tree, int,
+						 bool *);
+extern tree darwin_handle_availability_attribute (tree *, tree, tree,
+						  int, bool *);
+extern bool darwin_attribute_takes_identifier_p (const_tree);
+
 extern void machopic_output_stub (FILE *, const char *, const char *);
 extern void darwin_globalize_label (FILE *, const char *);
 extern void darwin_assemble_visibility (tree, int);
diff --git a/gcc/config/darwin.cc b/gcc/config/darwin.cc
index 621a94d74a2..8bb4a439996 100644
--- a/gcc/config/darwin.cc
+++ b/gcc/config/darwin.cc
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfghooks.h"
 #include "df.h"
 #include "memmodel.h"
+#include "c-family/c-common.h"  /* enum rid.  */
 #include "tm_p.h"
 #include "stringpool.h"
 #include "attribs.h"
@@ -49,6 +50,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "optabs.h"
 #include "flags.h"
 #include "opts.h"
+#include "c-family/c-objc.h"    /* for objc_method_decl().  */
 
 /* Fix and Continue.
 
@@ -102,6 +104,7 @@ int darwin_running_cxx;
 
 /* Some code-gen now depends on OS major version numbers (at least).  */
 int generating_for_darwin_version ;
+unsigned long current_os_version = 0;
 
 /* For older linkers we need to emit special sections (marked 'coalesced') for
    for weak or single-definition items.  */
@@ -2181,6 +2184,96 @@ darwin_handle_kext_attribute (tree *node, tree name,
   return NULL_TREE;
 }
 
+enum version_components { MAJOR, MINOR, TINY };
+
+/* Parse a version number in x.y.z form and validate it as a macOS
+   version.  Ideally, we'd put this in a common place usable by the
+   Darwin backend.  */
+
+static bool
+parse_version (unsigned version_array[3], const char *version_str)
+{
+  size_t version_len;
+  char *end;
+
+  version_len = strlen (version_str);
+  if (version_len < 1)
+    return false;
+
+  /* Version string must consist of digits and periods only.  */
+  if (strspn (version_str, "0123456789.") != version_len)
+    return false;
+
+  if (!ISDIGIT (version_str[0]) || !ISDIGIT (version_str[version_len - 1]))
+    return false;
+
+  version_array[MAJOR] = strtoul (version_str, &end, 10);
+  version_str = end + ((*end == '.') ? 1 : 0);
+  if (version_array[MAJOR]  > 99)
+    return false;
+
+  /* Version string must not contain adjacent periods.  */
+  if (*version_str == '.')
+    return false;
+
+  version_array[MINOR] = strtoul (version_str, &end, 10);
+  version_str = end + ((*end == '.') ? 1 : 0);
+  if (version_array[MINOR]  > 99)
+    return false;
+
+  version_array[TINY] = strtoul (version_str, &end, 10);
+  if (version_array[TINY]  > 99)
+    return false;
+
+  /* Version string must contain no more than three tokens.  */
+  if (*end != '\0')
+    return false;
+
+  return true;
+}
+
+/* Turn a version expressed as maj.min.tiny into an unsigned long
+   integer representing the value used in macOS availability macros.  */
+
+static unsigned long
+version_from_version_array (unsigned vers[3])
+{
+  unsigned long res = 0;
+  /* Here, we follow the 'modern' / 'legacy' numbering scheme for versions.  */
+  if (vers[0] > 10 || vers[1] >= 10)
+    res = vers[0] * 10000 + vers[1] * 100 + vers[2];
+  else
+    {
+      res = vers[0] * 100;
+      if (vers[1] > 9)
+	res += 90;
+      else
+	res += vers[1] * 10;
+      if (vers[2] > 9)
+	res += 9;
+      else
+	res += vers[1];
+    }
+  return res;
+}
+
+/* Extract a macOS version from an availability attribute argument.  */
+
+static unsigned long
+os_version_from_avail_value (tree value)
+{
+  unsigned long res = 0;
+  unsigned vers[3] = {0,0,0};
+  if (TREE_CODE (value) == STRING_CST)
+    {
+      if (parse_version (&vers[0], TREE_STRING_POINTER (value)))
+	res = version_from_version_array (&vers[0]);
+    }
+  else
+    gcc_unreachable ();
+  return res;
+}
+
 /* Handle a "weak_import" attribute; arguments as in
    struct attribute_spec.handler.  */
 
@@ -2202,6 +2295,249 @@ darwin_handle_weak_import_attribute (tree *node, tree name,
   return NULL_TREE;
 }
 
+#define NUM_AV_OSES 12
+const char *availability_os[NUM_AV_OSES]
+  = { "macos", "macosx", "ios", "tvos", "watchos", "driverkit", "swift",
+      "maccatalyst", "xros", "visionos", "android", "zos" };
+
+#define NUM_AV_CLAUSES 6
+const char *availability_clause[NUM_AV_CLAUSES]
+  = { "unavailable", "introduced", "deprecated", "obsoleted", "message",
+      "replacement" };
+
+/* Validate and act upon the arguments to an 'availability' attribute.  */
+
+tree
+darwin_handle_availability_attribute (tree *node, tree name, tree args,
+				      int flags, bool * no_add_attrs)
+{
+  tree decl = *node;
+
+  if (! VAR_OR_FUNCTION_DECL_P (decl))
+    {
+      warning (OPT_Wattributes, "%qE attribute ignored",
+	       name);
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+
+  location_t loc = DECL_SOURCE_LOCATION (decl);
+  if (args == NULL_TREE)
+    {
+      error_at (loc, "%qE attribute requires at least one argument",
+		name);
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+
+  /* The first argument must name a supported OS - although we could choose
+     to ignore any OS we don't recognise.  */
+  gcc_checking_assert (TREE_CODE (args) == TREE_LIST);
+  tree platform = TREE_VALUE (args);
+  gcc_checking_assert (TREE_CODE (platform) == IDENTIFIER_NODE);
+  bool platform_ok = false;
+  unsigned plat_num = 0;
+  for (; plat_num < (unsigned) NUM_AV_OSES; plat_num++)
+    if (strcmp (availability_os[plat_num], IDENTIFIER_POINTER (platform)) == 0)
+      {
+	platform_ok = true;
+	break;
+      }
+  if (!platform_ok)
+    {
+      error_at (input_location,
+		"platform %qE is not recognised for the %<availability%> "
+		"attribute", platform);
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+  else if (plat_num > 1) /* We only compile for macos so far.  */
+    {
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+
+  /* We might be dealing with an object or type.  */
+  tree target_decl = NULL_TREE;
+  tree type = NULL_TREE;
+  bool warn = false;
+  if (DECL_P (*node))
+    {
+      type = TREE_TYPE (decl);
+
+      if (TREE_CODE (decl) == TYPE_DECL
+	  || TREE_CODE (decl) == PARM_DECL
+	  || VAR_OR_FUNCTION_DECL_P (decl)
+	  || TREE_CODE (decl) == FIELD_DECL
+	  || TREE_CODE (decl) == CONST_DECL
+	  /*|| objc_method_decl (TREE_CODE (decl))*/)
+	target_decl = decl;
+      else
+	warn = true;
+    }
+  else if (TYPE_P (*node))
+    type = target_decl = *node;
+  else
+    warn = true;
+
+  tree what = NULL_TREE;
+  if (warn)
+    {
+      *no_add_attrs = true;
+      if (type && TYPE_NAME (type))
+	{
+	  if (TREE_CODE (TYPE_NAME (type)) == IDENTIFIER_NODE)
+	    what = TYPE_NAME (*node);
+	  else if (TREE_CODE (TYPE_NAME (type)) == TYPE_DECL
+		   && DECL_NAME (TYPE_NAME (type)))
+	    what = DECL_NAME (TYPE_NAME (type));
+	}
+      if (what)
+	warning (OPT_Wattributes, "%qE attribute ignored for %qE", name, what);
+      else
+	warning (OPT_Wattributes, "%qE attribute ignored", name);
+      return NULL_TREE;
+    }
+
+  /* Now we have to parse the availability clauses.  */
+  tree msg = NULL_TREE;
+  tree replacement = NULL_TREE;
+  bool unavailable = false;
+  unsigned introduced = 1000;
+  unsigned deprecated = current_os_version + 1;
+  unsigned obsoleted = current_os_version + 1;
+  for (tree arg = TREE_CHAIN (args); arg; arg = TREE_CHAIN (arg))
+    {
+      tree clause_name = TREE_VALUE (arg);
+      tree clause_value = TREE_PURPOSE (arg);
+      unsigned clause_num = 0;
+      for (; clause_num < (unsigned) NUM_AV_CLAUSES; clause_num++)
+	if (strcmp (availability_clause[clause_num],
+		    IDENTIFIER_POINTER (clause_name)) == 0)
+	  break;
+      switch (clause_num)
+        {
+	default:
+	  {
+	    error_at (input_location,
+		      "clause %qE is not recognised for the %<availability%> "
+		      "attribute", clause_name);
+	    *no_add_attrs = true;
+	    return NULL_TREE;
+	  }
+	  break;
+	case 0:
+	  unavailable = true;
+	  break;
+	case 1:
+	case 2:
+	case 3:
+	  if (!clause_value)
+	    {
+	      error ("%qs= requires a value",
+		     clause_num > 2 ? "obsoleted"
+				    : clause_num > 1 ? "deprecated"
+						     : "introduced");
+	      *no_add_attrs = true;
+	    }
+	  else
+	    {
+	      unsigned version = os_version_from_avail_value (clause_value);
+	      if (version == 0)
+		{
+		  error ("the value %qE provided to %qs is not a "
+			 "valid OS version", clause_value,
+			 (clause_num > 2 ? "obsoleted"
+					 : clause_num > 1 ? "deprecated"
+							  : "introduced"));
+		  *no_add_attrs = true;
+		}
+	      else if (clause_num == 1)
+		introduced = version;
+	      else if (clause_num == 2)
+		deprecated = version;
+	      else if (clause_num == 3)
+		obsoleted = version;
+	    }
+	  break;
+	case 4:
+	  if (TREE_CODE (clause_value) != STRING_CST)
+	    {
+	      error ("%<message=%> requires a string");
+	      *no_add_attrs = true;
+	    }
+	  else
+	    msg = clause_value;
+	  break;
+	case 5:
+	  if (TREE_CODE (clause_value) != STRING_CST)
+	    {
+	      error ("%<replacement=%> requires a string");
+	      *no_add_attrs = true;
+	    }
+	  else
+	    replacement = clause_value;
+	  break;
+	}
+    }
+  /* Now figure out what to do.  */
+  if (unavailable
+      || current_os_version >= obsoleted
+      || current_os_version < introduced)
+    {
+      tree new_attr = NULL_TREE;
+      if (replacement)
+	new_attr
+	  = tree_cons (get_identifier ("unavailable"),
+		       tree_cons (NULL_TREE, replacement, NULL_TREE), NULL_TREE);
+      else if (msg)
+	new_attr
+	  = tree_cons (get_identifier ("unavailable"),
+		       tree_cons (NULL_TREE, msg, NULL_TREE), NULL_TREE);
+      /* This is the actual consequence of the evaluation.  */
+      if (TYPE_P (target_decl) && !(flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
+	{
+	  *node = build_variant_type_copy (*node);
+	  TYPE_ATTRIBUTES (*node) = chainon (TYPE_ATTRIBUTES (*node), new_attr);
+	}
+      else
+	DECL_ATTRIBUTES (*node) = chainon (DECL_ATTRIBUTES (*node), new_attr);
+//      *no_add_attrs = true; /* maybe we should add to detect conflicts.  */
+      TREE_UNAVAILABLE (*node) = true;
+      return NULL_TREE;
+    }
+   else if (current_os_version > deprecated)
+    {
+      tree new_attr = NULL_TREE;
+      if (replacement)
+	new_attr
+	  = tree_cons (get_identifier ("deprecated"),
+		       tree_cons (NULL_TREE, replacement, NULL_TREE), NULL_TREE);
+      else if (msg)
+	new_attr
+	  = tree_cons (get_identifier ("deprecated"),
+		       tree_cons (NULL_TREE, msg, NULL_TREE), NULL_TREE);
+      /* This is the actual consequence of the evaluation.  */
+      if (TYPE_P (target_decl) && !(flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
+	{
+	  *node = build_variant_type_copy (*node);
+	  TYPE_ATTRIBUTES (*node) = chainon (TYPE_ATTRIBUTES (*node), new_attr);
+	}
+      else
+	DECL_ATTRIBUTES (*node) = chainon (DECL_ATTRIBUTES (*node), new_attr);
+//      *no_add_attrs = true; /* maybe we should add to detect conflicts.  */
+      TREE_DEPRECATED (*node) = true;
+      return NULL_TREE;
+    }
+ return NULL_TREE;
+}
+
+bool
+darwin_attribute_takes_identifier_p (const_tree attr_id)
+{
+  return is_attribute_p ("availability", attr_id);
+}
+
 /* Emit a label for an FDE, making it global and/or weak if appropriate.
    The third parameter is nonzero if this is for exception handling.
    The fourth parameter is nonzero if this is just a placeholder for an
@@ -3337,7 +3673,14 @@ darwin_override_options (void)
 	generating_for_darwin_version = 8;
 
       /* Earlier versions are not specifically accounted, until required.  */
+      unsigned vers[3] = {0,0,0};
+      if (!parse_version (vers, darwin_macosx_version_min))
+	error_at (UNKNOWN_LOCATION, "how did we get a bad OS version? (%s)",
+		  darwin_macosx_version_min);
+      current_os_version = version_from_version_array (vers);
     }
+  else
+    current_os_version = 1058;
 
   /* Some codegen needs to account for the capabilities of the target
      linker.  */
diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
index 5db64a1ad68..684c754ce0f 100644
--- a/gcc/config/darwin.h
+++ b/gcc/config/darwin.h
@@ -963,7 +963,12 @@ extern GTY(()) section * darwin_sections[NUM_DARWIN_SECTIONS];
   { "apple_kext_compatibility", 0, 0, false, true, false, false,	     \
     darwin_handle_kext_attribute, NULL },				     \
   { "weak_import", 0, 0, true, false, false, false,			     \
-    darwin_handle_weak_import_attribute, NULL }
+    darwin_handle_weak_import_attribute, NULL },			     \
+  { "availability", 0, -1, true, false, false, false,			     \
+    darwin_handle_availability_attribute, NULL }
+
+#undef TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P
+#define TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P darwin_attribute_takes_identifier_p
 
 /* Make local constant labels linker-visible, so that if one follows a
    weak_global constant, ld64 will be able to separate the atoms.  */
-- 
2.39.2 (Apple Git-143)

Re: [PATCH 1/4] c-family: Add handling for clang-style attributes [PR109877].

[Jeff Law]

On 11/12/23 23:02, Iain Sandoe wrote:
> This patch set is not actually particualry new, I have been maintaining
> it locally one Darwin branches and it has been tested on several versions
> of Darwin both with and without Alex's __has_{feature, extension} patch.
> 
> This is one of the three most significant blockers to importing the macOS
> SDKs properly, and cannot currently be fixincludes-ed (in fact it can not
> ever really since the attribute is uaer-facing and so can be in end-user
> code that we cannot fix).
> 
> OK for trunk?
> thanks
> Iain
> 
> --- 8< ---
> 
> 
> The clang compiler supports essentially arbitrary, per-attribute, syntax and
> token forms for attribute arguments.  This extends to the case where token
> forms are required to be accepted that are not part of the valid set for
> standard C or C++.
> 
> A motivating  example (in the initial attribute of this form implemented
> in this patch set) is version-style (i.e. x.y.z) numeric values.  At present
> the c-family cannot handle this, since invalid numeric tokens are rejected
> by both C and C++ frontends before we have a chance to decide to accept them
> in custom attribute argument parsing.
> 
> The solution proposed in this patch series is to allow for a certain set of
> attributes names that are known to be 'clang-form' and to defer argument
> token validation until the parse of those arguments.
> 
> This does not apparently represent any loss of generality - since the
> specific attribute names are already claimed by clang and re-using them with
> different semantics in GCC would be a highly unfortunate experience for end-
> users.
> 
> The first patch here adds a mechanism to check attribute identifiers against
> a list known to be in clang form.  The 'availability' attribute is added as a
> first example.
> 
> The acceptance of non-standard tokens is constrained to the interval enclosing
> the attribute arguments of cases notified as 'clang-form'.
> 
> 	PR c++/109877
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-attribs.cc (attribute_clang_form_p): New.
> 	* c-common.h (attribute_clang_form_p): New.
Patches #1-#3 are fine if nobody has objected within say 48hrs. 
Basically I agree that we have to do something along the lines of what 
you're suggesting and I just want to give folks the opportunity to raise 
any implementation issues they may see.

Patch #4 is obviously darwin specific and I think you can self-approve.


Jeff

Re: [PATCH 2/4] c-family, C: handle clang attributes [PR109877].

[Marek Polacek]

On Sun, Nov 12, 2023 at 08:02:42PM -1000, Iain Sandoe wrote:
> This adds the ability to defer the validation of numeric attribute
> arguments until the sequence is parsed if the attribute being
> handled is one known to be 'clang form'.
> 
> We do this by considering the arguments to be strings regardless
> of content and defer the interpretation of those strings until the
> argument processing.

I don't see any tests here nor in the C++ part of the patch.  Is it
possible to add some (I suppose for now only attribute availability)?

FWIW, for chaining attributes it's best to use attr_chainon since that
handles error_mark_node.  Unfortunately that's currently only in cp/.
 
> 	PR c++/109877
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-lex.cc (c_lex_with_flags): Allow for the case where
> 	we wish to defer interpretation of numeric values until
> 	parse time.
> 	* c-pragma.h (C_LEX_NUMBER_AS_STRING): New.
> 
> gcc/c/ChangeLog:
> 
> 	* c-parser.cc (struct c_parser): Provide a flag to notify
>         that argument parsing should return attribute arguments
>         as string constants.
> 	(c_lex_one_token): Act to defer numeric value validation.
> 	(c_parser_clang_attribute_arguments): New.
> 	(c_parser_gnu_attribute): Allow for clang-form GNU-style
> 	attributes.
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> ---
>  gcc/c-family/c-lex.cc   |  15 ++++++
>  gcc/c-family/c-pragma.h |   3 ++
>  gcc/c/c-parser.cc       | 109 ++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 122 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/c-family/c-lex.cc b/gcc/c-family/c-lex.cc
> index 06c2453c89a..d535f5b460c 100644
> --- a/gcc/c-family/c-lex.cc
> +++ b/gcc/c-family/c-lex.cc
> @@ -533,6 +533,21 @@ c_lex_with_flags (tree *value, location_t *loc, unsigned char *cpp_flags,
>  
>      case CPP_NUMBER:
>        {
> +	/* If the user wants number-like entities to be returned as a raw
> +	   string, then don't try to classify them, which emits unwanted
> +	   diagnostics.  */
> +	if (lex_flags & C_LEX_NUMBER_AS_STRING)
> +	  {
> +	    /* build_string adds a trailing NUL at [len].  */
> +	    tree num_string = build_string (tok->val.str.len + 1,
> +					    (const char *) tok->val.str.text);
> +	    TREE_TYPE (num_string) = char_array_type_node;
> +	    *value = num_string;
> +	    /* We will effectively note this as CPP_N_INVALID, because we
> +	       made no checks here.  */
> +	    break;
> +	  }
> +
>  	const char *suffix = NULL;
>  	unsigned int flags = cpp_classify_number (parse_in, tok, &suffix, *loc);
>  
> diff --git a/gcc/c-family/c-pragma.h b/gcc/c-family/c-pragma.h
> index 98177913053..11cde74f9f0 100644
> --- a/gcc/c-family/c-pragma.h
> +++ b/gcc/c-family/c-pragma.h
> @@ -276,6 +276,9 @@ extern void pragma_lex_discard_to_eol ();
>  #define C_LEX_STRING_NO_JOIN	  2 /* Do not concatenate strings
>  				       nor translate them into execution
>  				       character set.  */
> +#define C_LEX_NUMBER_AS_STRING	  4 /* Do not classify a number, but
> +				       instead return it as a raw
> +				       string.  */
>  
>  /* This is not actually available to pragma parsers.  It's merely a
>     convenient location to declare this function for c-lex, after
> diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
> index 703f9570dbc..aaaa16cc05d 100644
> --- a/gcc/c/c-parser.cc
> +++ b/gcc/c/c-parser.cc
> @@ -217,6 +217,9 @@ struct GTY(()) c_parser {
>       should translate them to the execution character set (false
>       inside attributes).  */
>    BOOL_BITFIELD translate_strings_p : 1;
> +  /* True if we want to lex arbitrary number-like sequences as their
> +     string representation.  */
> +  BOOL_BITFIELD lex_number_as_string : 1;
>  
>    /* Objective-C specific parser/lexer information.  */
>  
> @@ -308,10 +311,10 @@ c_lex_one_token (c_parser *parser, c_token *token, bool raw = false)
>  
>    if (raw || vec_safe_length (parser->raw_tokens) == 0)
>      {
> +      int lex_flags = parser->lex_joined_string ? 0 : C_LEX_STRING_NO_JOIN;
> +      lex_flags |= parser->lex_number_as_string ? C_LEX_NUMBER_AS_STRING : 0;
>        token->type = c_lex_with_flags (&token->value, &token->location,
> -				      &token->flags,
> -				      (parser->lex_joined_string
> -				       ? 0 : C_LEX_STRING_NO_JOIN));
> +				      &token->flags, lex_flags);
>        token->id_kind = C_ID_NONE;
>        token->keyword = RID_MAX;
>        token->pragma_kind = PRAGMA_NONE;
> @@ -5210,6 +5213,98 @@ c_parser_gnu_attribute_any_word (c_parser *parser)
>    return attr_name;
>  }
>  
> +/* Handle parsing clang-form attribute arguments, where we need to adjust
> +   the parsing rules to relate to a specific attribute.  */
> +
> +static tree
> +c_parser_clang_attribute_arguments (c_parser *parser, tree /*attr_id*/)

Why the second parameter if you don't use it?

> +{
> +  /* We can, if required, alter the parsing on the basis of the attribute.
> +     At present, we handle the availability attr, where ach entry can be :

"each"

> +	identifier
> +	identifier=N.MM.Z
> +	identifier="string"
> +	followed by ',' or ) for the last entry*/

".  */"

> +
> +  tree attr_args = NULL_TREE;
> +  if (c_parser_next_token_is (parser, CPP_NAME)
> +      && c_parser_peek_token (parser)->id_kind == C_ID_ID
> +      && c_parser_peek_2nd_token (parser)->type == CPP_COMMA)
> +    {
> +      tree platf = c_parser_peek_token (parser)->value;
> +      c_parser_consume_token (parser);
> +      attr_args = tree_cons (NULL_TREE, platf, NULL_TREE);
> +    }
> +  else
> +    {
> +      c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
> +				"expected a platform name followed by %<,%>");
> +      return error_mark_node;
> +    }
> +  c_parser_consume_token (parser); /* consume the ',' */
> +  do
> +    {
> +      tree name = NULL_TREE;
> +      tree value = NULL_TREE;
> +
> +      if (c_parser_next_token_is (parser, CPP_NAME)
> +	  && c_parser_peek_token (parser)->id_kind == C_ID_ID)
> +	{
> +	  name = c_parser_peek_token (parser)->value;
> +	  c_parser_consume_token (parser);
> +	}
> +      else
> +	{
> +	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
> +				     "expected an attribute keyword");
> +	  return error_mark_node;
> +	}
> +      if (c_parser_next_token_is (parser, CPP_EQ))
> +	{
> +	  c_parser_consume_token (parser); /* eat the '=' */
> +	  /* We need to bludgeon the lexer into not trying to interpret the
> +	     xx.yy.zz form, since that just looks like a malformed float.
> +	     Also, as a result of macro processing, we can have strig literals

"string"

> +	     that are in multiple pieces so, for this specific part of the
> +	     parse, we need to join strings.  */
> +	  bool saved_join_state = parser->lex_joined_string;
> +	  parser->lex_number_as_string = 1;
> +	  parser->lex_joined_string = 1;
> +	  /* So look at the next token, expecting a string, or something that
> +	     looks initially like a number, but might be a version number.  */
> +	  c_parser_peek_token (parser);
> +	  /* Done with the funky number parsing.  */
> +	  parser->lex_number_as_string = 0;
> +	  parser->lex_joined_string = saved_join_state;
> +	  if (c_parser_next_token_is_not (parser, CPP_CLOSE_PAREN)
> +	      && c_parser_next_token_is_not (parser, CPP_COMMA))
> +	    {
> +	      value = c_parser_peek_token (parser)->value;
> +	      /* ???: check for error mark and early-return?  */

It might be useful to have a test for this invalid case.

> +	      c_parser_consume_token (parser);
> +	    }
> +	  else
> +	    {
> +	      c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
> +					 "expected a value");
> +	      return error_mark_node;
> +	    }
> +	}
> +      else if (c_parser_next_token_is_not (parser, CPP_CLOSE_PAREN)
> +	       && c_parser_next_token_is_not (parser, CPP_COMMA))
> +	{
> +	  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
> +				     "expected %<,%> or %<=%>");
> +	  return error_mark_node;
> +	}
> +    if (c_parser_next_token_is (parser, CPP_COMMA))
> +      c_parser_consume_token (parser); /* Just skip the comma.  */
> +    tree t = tree_cons (value, name, NULL);
> +    chainon (attr_args, t);
> +  } while (c_parser_next_token_is_not (parser, CPP_CLOSE_PAREN));
> +  return attr_args;
> +}
> +
>  /* Parse attribute arguments.  This is a common form of syntax
>     covering all currently valid GNU and standard attributes.
>  
> @@ -5375,9 +5470,13 @@ c_parser_gnu_attribute (c_parser *parser, tree attrs,
>        attrs = chainon (attrs, attr);
>        return attrs;
>      }
> -  c_parser_consume_token (parser);
> +  c_parser_consume_token (parser); /* The '('.  */
>  
> -  tree attr_args
> +  tree attr_args;
> +  if (attribute_clang_form_p (attr_name))
> +    attr_args = c_parser_clang_attribute_arguments (parser, attr_name);
> +  else
> +    attr_args
>      = c_parser_attribute_arguments (parser,
>  				    attribute_takes_identifier_p (attr_name),
>  				    false,
> -- 
> 2.39.2 (Apple Git-143)
> 

Marek

Re: [PATCH 2/4] c-family, C: handle clang attributes [PR109877].

[Joseph Myers]

On Sun, 12 Nov 2023, Iain Sandoe wrote:

> This adds the ability to defer the validation of numeric attribute
> arguments until the sequence is parsed if the attribute being
> handled is one known to be 'clang form'.

This is only for __attribute__ and not [[]]-style attributes, is that as 
intended?  (Doing it for [[]] might be harder because of how the tokens 
after [[ have to be lexed early for Objective-C to see whether there is a 
matching ]] and thus whether it's an attribute at all.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 1/4] c-family: Add handling for clang-style attributes [PR109877].

[Jason Merrill]

On 11/13/23 01:02, Iain Sandoe wrote:
> The clang compiler supports essentially arbitrary, per-attribute, syntax and
> token forms for attribute arguments.  This extends to the case where token
> forms are required to be accepted that are not part of the valid set for
> standard C or C++.
> 
> A motivating  example (in the initial attribute of this form implemented
> in this patch set) is version-style (i.e. x.y.z) numeric values.  At present
> the c-family cannot handle this, since invalid numeric tokens are rejected
> by both C and C++ frontends before we have a chance to decide to accept them
> in custom attribute argument parsing.
> 
> The solution proposed in this patch series is to allow for a certain set of
> attributes names that are known to be 'clang-form' and to defer argument
> token validation until the parse of those arguments.
> 
> This does not apparently represent any loss of generality - since the
> specific attribute names are already claimed by clang and re-using them with
> different semantics in GCC would be a highly unfortunate experience for end-
> users.
> 
> The first patch here adds a mechanism to check attribute identifiers against
> a list known to be in clang form.  The 'availability' attribute is added as a
> first example.
> 
> The acceptance of non-standard tokens is constrained to the interval enclosing
> the attribute arguments of cases notified as 'clang-form'.

I don't love describing this category as "clang form", I'd prefer 
something more descriptive that fits better with the other attribute 
argument parsing variants we already have:

> /* Values for the second parameter of cp_parser_parenthesized_expression_list.  */
> enum { non_attr = 0, normal_attr = 1, id_attr = 2, assume_attr = 3,
>        uneval_string_attr = 4 };

Maybe version_num_attr for this?

Jason

Re: [PATCH 3/4] c-family, C++: Handle clang attributes [PR109877].

[Marek Polacek]

On Sun, Nov 12, 2023 at 08:02:43PM -1000, Iain Sandoe wrote:
> This adds the ability to defer the validation of numeric attribute
> arguments until the sequence is parsed if the attribute being
> handled is one known to be 'clang form'.
> 
> We do this by considering the arguments to be strings regardless
> of content and defer the interpretation of those strings until the
> argument processing.
> 
> Since the C++ front end lexes tokens separately (and would report
> non-compliant numeric tokens before we can intercept them), we need
> to implement a small state machine to track the lexing of attributes.
> 
> 	PR c++/109877
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (enum clang_attr_state): New.
> 	(cp_lexer_attribute_state): New.
> 	(cp_lexer_new_main): Set initial attribute lexing state.
> 	(cp_lexer_get_preprocessor_token): Handle lexing of clang-
> 	form attributes.
> 	(cp_parser_clang_attribute): Handle clang-form attributes.
> 	(cp_parser_gnu_attribute_list): Switch to clang-form parsing
> 	where needed.
> 	* parser.h : New flag to signal lexing clang-form attributes.
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> ---
>  gcc/cp/parser.cc | 230 +++++++++++++++++++++++++++++++++++++++++++++--
>  gcc/cp/parser.h  |   3 +
>  2 files changed, 227 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 5116bcb78f6..c12f473f2e3 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -699,6 +699,91 @@ cp_lexer_handle_early_pragma (cp_lexer *lexer)
>  static cp_parser *cp_parser_new (cp_lexer *);
>  static GTY (()) cp_parser *the_parser;
>  
> +/* Context-sensitive parse-checking for clang-style attributes.  */
> +
> +enum clang_attr_state {
> +  CA_NONE = 0,
> +  CA_ATTR,
> +  CA_BR1, CA_BR2,
> +  CA_LIST,
> +  CA_LIST_ARGS,
> +  CA_IS_CA,
> +  CA_CA_ARGS,
> +  CA_LIST_CONT
> +};
> +
> +/* State machine tracking context of attribute lexing.  */
> +
> +static enum clang_attr_state
> +cp_lexer_attribute_state (cp_token& token, enum clang_attr_state attr_state)
> +{
> +  /* Implement a context-sensitive parser for clang attributes.
> +     We detect __attribute__((clang_style_attribute (ARGS))) and lex the
> +     args ARGS with the following differences from GNU attributes:
> +	(a) number-like values are lexed as strings [this allows lexing XX.YY.ZZ
> +	   version numbers].
> +	(b) we concatenate strings, since clang attributes allow this too.  */
> +  switch (attr_state)
> +    {
> +    case CA_NONE:
> +      if (token.type == CPP_KEYWORD
> +	  && token.keyword == RID_ATTRIBUTE)
> +	attr_state = CA_ATTR;
> +      break;
> +    case CA_ATTR:
> +      if (token.type == CPP_OPEN_PAREN)
> +	attr_state = CA_BR1;
> +      else
> +	attr_state = CA_NONE;
> +      break;
> +    case CA_BR1:
> +      if (token.type == CPP_OPEN_PAREN)
> +	attr_state = CA_BR2;
> +      else
> +	attr_state = CA_NONE;
> +      break;
> +    case CA_BR2:
> +      if (token.type == CPP_NAME)
> +	{
> +	  tree identifier = (token.type == CPP_KEYWORD)
> +	    /* For keywords, use the canonical spelling, not the
> +	       parsed identifier.  */
> +	    ? ridpointers[(int) token.keyword]
> +	    : token.u.value;
> +	  identifier = canonicalize_attr_name (identifier);
> +	  if (attribute_clang_form_p (identifier))
> +	    attr_state = CA_IS_CA;
> +	  else
> +	    attr_state = CA_LIST;
> +	}
> +      else
> +	attr_state = CA_NONE;
> +      break;
> +    case CA_IS_CA:
> +    case CA_LIST:
> +      if (token.type == CPP_COMMA)
> +	attr_state = CA_BR2; /* Back to the list outer.  */
> +      else if (token.type == CPP_OPEN_PAREN)
> +	attr_state = attr_state == CA_IS_CA ? CA_CA_ARGS
> +					    : CA_LIST_ARGS;
> +      else
> +	attr_state = CA_NONE;
> +      break;
> +    case CA_CA_ARGS: /* We will special-case args in this state.  */
> +    case CA_LIST_ARGS:
> +      if (token.type == CPP_CLOSE_PAREN)
> +	attr_state = CA_LIST_CONT;
> +      break;
> +    case CA_LIST_CONT:
> +      if (token.type == CPP_COMMA)
> +	attr_state = CA_BR2; /* Back to the list outer.  */
> +      else
> +	attr_state = CA_NONE;
> +      break;
> +    }
> +  return attr_state;
> +}
> +
>  /* Create a new main C++ lexer, the lexer that gets tokens from the
>     preprocessor, and also create the main parser.  */
>  
> @@ -715,6 +800,9 @@ cp_lexer_new_main (void)
>    c_common_no_more_pch ();
>  
>    cp_lexer *lexer = cp_lexer_alloc ();
> +  lexer->lex_number_as_string_p = false;
> +  enum clang_attr_state attr_state = CA_NONE;
> +
>    /* Put the first token in the buffer.  */
>    cp_token *tok = lexer->buffer->quick_push (token);
>  
> @@ -738,8 +826,20 @@ cp_lexer_new_main (void)
>        if (tok->type == CPP_PRAGMA_EOL)
>  	cp_lexer_handle_early_pragma (lexer);
>  
> +      attr_state = cp_lexer_attribute_state (*tok, attr_state);
>        tok = vec_safe_push (lexer->buffer, cp_token ());
> -      cp_lexer_get_preprocessor_token (C_LEX_STRING_NO_JOIN, tok);
> +      unsigned int flags = C_LEX_STRING_NO_JOIN;
> +      if (attr_state == CA_CA_ARGS)
> +	{
> +	  /* We are handling a clang attribute;
> +	     1. do not try to diagnose x.y.z as a bad number, it could be a
> +		version.
> +	     2. join strings.  */
> +          flags = C_LEX_NUMBER_AS_STRING;
> +	  lexer->lex_number_as_string_p = true;
> +	}
> +      cp_lexer_get_preprocessor_token (flags, tok);
> +      lexer->lex_number_as_string_p = false;
>      }
>  
>    lexer->next_token = lexer->buffer->address ();
> @@ -956,7 +1056,15 @@ cp_lexer_get_preprocessor_token (unsigned flags, cp_token *token)
>  {
>    static int is_extern_c = 0;
>  
> -   /* Get a new token from the preprocessor.  */
> +   /* Get a new token from the preprocessor.
> +  int lex_flags = 0;
> +  if (lexer != NULL)
> +    {
> +      lex_flags = C_LEX_STRING_NO_JOIN;
> +      if (lexer->lex_number_as_string_p)
> +	lex_flags |= C_LEX_NUMBER_AS_STRING;
> +    }
> +  */

Why is this commented out?

>    token->type
>      = c_lex_with_flags (&token->u.value, &token->location, &token->flags,
>  			flags);
> @@ -29302,6 +29410,113 @@ cp_parser_gnu_attributes_opt (cp_parser* parser)
>    return attributes;
>  }
>  
> +/* Parse the arguments list for a clang attribute.   */
> +static tree
> +cp_parser_clang_attribute (cp_parser *parser, tree/*attr_id*/)
> +{
> +  /* Each entry can be :
> +     identifier
> +     identifier=N.MM.Z
> +     identifier="string"
> +     followed by ',' or ) for the last entry*/
> +
> +  matching_parens parens;
> +  if (!parens.require_open (parser))
> +    return NULL;
> +
> +  bool save_translate_strings_p = parser->translate_strings_p;
> +  parser->translate_strings_p = false;

This could be

  auto ts = make_temp_override (parser->translate_strings_p, false);

and then...

> +  tree attr_args = NULL_TREE;
> +  cp_token *token = cp_lexer_peek_token (parser->lexer);
> +  if (token->type == CPP_NAME
> +      && cp_lexer_peek_nth_token (parser->lexer, 2)->type == CPP_COMMA)
> +    {
> +      tree platf = token->u.value;
> +      cp_lexer_consume_token (parser->lexer);
> +      attr_args = tree_cons (NULL_TREE, platf, NULL_TREE);
> +    }
> +  else
> +    {
> +      error_at (token->location,
> +		"expected a platform name followed by %<,%>");
> +      cp_parser_skip_to_closing_parenthesis (parser,
> +					     /*recovering=*/true,
> +					     /*or_comma=*/false,
> +					     /*consume_paren=*/true);
> +      parser->translate_strings_p = save_translate_strings_p;

you don't need this, and the one below.

> +      return error_mark_node;
> +    }
> +
> +  cp_lexer_consume_token (parser->lexer); /* consume the ',' */
> +  do
> +    {
> +      tree name = NULL_TREE;
> +      tree value = NULL_TREE;
> +
> +      token = cp_lexer_peek_token (parser->lexer);
> +      if (token->type == CPP_NAME)
> +	{
> +	  name = token->u.value;
> +	  cp_lexer_consume_token (parser->lexer);
> +	}
> +      else
> +	{
> +	  /* FIXME: context-sensitive for that attrib.  */
> +	  error_at (token->location, "expected an attribute keyword");
> +	  cp_parser_skip_to_closing_parenthesis (parser,
> +						 /*recovering=*/true,
> +						 /*or_comma=*/false,
> +						 /*consume_paren=*/true);
> +	  attr_args = error_mark_node;
> +	  break;
> +	}
> +
> +      if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
> +	{
> +	  cp_lexer_consume_token (parser->lexer); /* eat the '=' */
> +	  token = cp_lexer_peek_token (parser->lexer);
> +	  if (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN)
> +	      && cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
> +	    {
> +	      value = token->u.value;
> +	      /* ???: check for error mark and early-return?  */
> +	      cp_lexer_consume_token (parser->lexer);
> +	    }
> +	  else
> +	    {
> +	      error_at (token->location, "expected a value");
> +	      cp_parser_skip_to_closing_parenthesis (parser,
> +						     /*recovering=*/true,
> +						     /*or_comma=*/false,
> +						     /*consume_paren=*/true);
> +	      attr_args = error_mark_node;
> +	      break;
> +	    }
> +	}
> +      else if (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN)
> +	       && cp_lexer_next_token_is_not (parser->lexer, CPP_COMMA))
> +	{
> +	  error_at (token->location, "expected %<,%>, %<=%> or %<)%>");
> +	  cp_parser_skip_to_closing_parenthesis (parser,
> +						 /*recovering=*/true,
> +						 /*or_comma=*/false,
> +						 /*consume_paren=*/true);
> +	  attr_args = error_mark_node;
> +	  break;
> +	}
> +    if (cp_lexer_next_token_is (parser->lexer, CPP_COMMA))
> +      cp_lexer_consume_token (parser->lexer);
> +    tree t = tree_cons (value, name, NULL_TREE);
> +    attr_args = chainon (attr_args, t);

attr_chainon

> +  } while (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN));

I think we format do-while as

  do
    {
    }
  while (cond);

> +
> +  parser->translate_strings_p = save_translate_strings_p;
> +  if (!parens.require_close (parser))
> +    return error_mark_node;
> +
> +  return attr_args;
> +}
> +
>  /* Parse a GNU attribute-list.
>  
>     attribute-list:
> @@ -29361,9 +29576,12 @@ cp_parser_gnu_attribute_list (cp_parser* parser, bool exactly_one /* = false */)
>  
>  	  /* Peek at the next token.  */
>  	  token = cp_lexer_peek_token (parser->lexer);
> -	  /* If it's an `(', then parse the attribute arguments.  */
> -	  if (token->type == CPP_OPEN_PAREN)
> +	  if (token->type == CPP_OPEN_PAREN
> +	      && attribute_clang_form_p (identifier))
> +	    arguments = cp_parser_clang_attribute (parser, identifier);
> +	  else if (token->type == CPP_OPEN_PAREN)

I think it'd be nice to eliminate checking token->type == CPP_OPEN_PAREN
twice here, and I'd say it's worth it wrapping attribute_clang_form_p (...)
in UNLIKELY (), since it should be extremely rare to be true?

Marek