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] cilkplus array notation for C (clean, independent patchset, take 1)


Continuing the review for coding style...

> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c

> +extern bool contains_array_notation_expr (tree);
> +extern struct c_expr fix_array_notation_expr (location_t, enum tree_code,
> +					      struct c_expr);
> +extern tree fix_conditional_array_notations (tree);
> +extern tree expand_array_notation_exprs (tree);

Again, include appropriate headers; no extern function declarations like 
this in .c files.

> +	      error_at (c_parser_peek_token (parser)->location,
> +			"array notations cannot be used in declaration.");

No "." at end of diagnostic.

> +	  error_at (c_parser_peek_token (parser)->location,
> +		    "array notations cannot be used in declaration.");

Likewise.

> +		  error_at (loc, "array notations cannot be used in a "
> +			    "condition for a for-loop.");

Likewise.

> +static tree 
> +c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index, 
> +			 tree array_value)
> +{
> +  c_token *token = NULL;
> +  tree start_index = NULL_TREE, end_index = NULL_TREE, stride = NULL_TREE;
> +  tree value_tree = NULL_TREE, type = NULL_TREE, array_type = NULL_TREE;
> +  tree array_type_domain = NULL_TREE; 
> +  double_int x;
> +
> +  if (!array_value || array_value == error_mark_node)

Can NULL actually occur here?

> +  token = c_parser_peek_token (parser);
> +   
> +  if (token == NULL)

c_parser_peek_token can never return NULL (EOF is a CPP_EOF token).

> +	      error_at (loc, "start-index and length fields necessary for "
> +			"using array notations in pointers.");

No "." at end of diagnostic.

> +	      error_at (loc, "array notations cannot be used with function "
> +			"type.");

Likewise.

> +		      error_at (loc, "array notations cannot be used with "
> +				"function pointer arrays.");

Likewise.

> +	      error_at (loc, "start-index and length fields necessary for "
> +			"using array notations in dimensionless arrays.");

Likewise.

> +	      error_at (loc, "start-index and length fields necessary for "
> +			"using array notations in variable-length arrays.");

Likewise.

> +	      c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, NULL);
> +	      return error_mark_node;
> +	    }
> +	  x = TREE_INT_CST (TYPE_MAXVAL (array_type_domain));
> +	  x.low++;

If you want to increment a double_int value, use operator ++ on the 
double_int itself; don't just change the low part and ignore possible 
overflow.

> +	      error_at (loc, "array notations cannot be used with function "
> +			"type.");

No "." at end of diagnostic.

> +		      error_at (loc, "array notations cannot be used with "
> +				"function pointer arrays.");

Likewise.

> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index ddb6d39..15dc83d 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -103,6 +103,8 @@ static void readonly_warning (tree, enum lvalue_use);
>  static int lvalue_or_else (location_t, const_tree, enum lvalue_use);
>  static void record_maybe_used_decl (tree);
>  static int comptypes_internal (const_tree, const_tree, bool *, bool *);
> +extern bool contains_array_notation_expr (tree);
> +extern tree find_correct_array_notation_type (tree);

Include headers, don't add extern function declarations in .c files.

> @@ -2303,7 +2305,17 @@ build_array_ref (location_t loc, tree array, tree index)
>    if (TREE_TYPE (array) == error_mark_node
>        || TREE_TYPE (index) == error_mark_node)
>      return error_mark_node;
> -
> +  

Don't change a blank line by adding trailing whitespace to it.

> +	  error_at (loc, "rank of the array's index is greater than 1.");

No "." at end of diagnostic.

> +      if (flag_enable_cilkplus && name
> +	  && !strncmp (IDENTIFIER_POINTER (name), "__sec_reduce", 12))

Should replace by something cleaner than examining the name, once the way 
these built-ins are implemented has been changed to have enum values.

> +      if (flag_enable_cilkplus && fundecl && DECL_NAME (fundecl)
> +	  && !strncmp (IDENTIFIER_POINTER (DECL_NAME (fundecl)),
> +		       "__sec_reduce", 12))

Likewise.

> +      /* If array notation is used and Cilk Plus is enabled, then we do not
> +	 worry about this error now.  We will handle them in a later place.  */
> +      if (flag_enable_cilkplus && DECL_NAME (fundecl)
> +	  && !strncmp (IDENTIFIER_POINTER (DECL_NAME (fundecl)), "__sec_reduce",
> +		       12))

Likewise.

> @@ -5097,7 +5130,6 @@ convert_for_assignment (location_t location, tree type, tree rhs,
>    enum tree_code coder;
>    tree rname = NULL_TREE;
>    bool objc_ok = false;
> -
>    if (errtype == ic_argpass)
>      {
>        tree selector;

Avoid spurious changes like this not related to the substance of the 
patch.

> +  if (flag_enable_cilkplus && contains_array_notation_expr (cond))
> +    {
> +      error_at (start_locus, "array notation expression cannot be used in a "
> +		"loop's condition");
> +      return;

Use %' for English apostrophes in diagnostics, so that a proper apostrophe 
rather than neutral vertical ' can be used in appropriate locales.

> +      error_at (start_locus, "array notation expression cannot be used in a "
> +		"loop's increment expression.");

Likewise, and remove the trailing ".".

-- 
Joseph S. Myers
joseph@codesourcery.com


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