Re: [patch,doc] Fix intrinsic arguments names

[FX <fxcoudert at gmail dot com>]

> some remarks; it is a bit chaotic since I first didn't see your texi
> changes. I will have another later.

Does that mean you have more? Please see below my remarks, an updated  
version of the patch, and a request for approval :)


> +    *vl = "values", *p1 = "path1", *p2 = "path2", *com = "command";
> [...]
> +  add_sym_2 ("link", GFC_ISYM_LINK, NO_CLASS, ACTUAL_NO,  
> BT_INTEGER, di,
> +	     GFC_STD_GNU, gfc_check_link, NULL, gfc_resolve_link,
> +	     "path1", BT_CHARACTER, dc, REQUIRED,
> +	     "path2", BT_CHARACTER, dc, REQUIRED);
>
> Is there any reason behind that you add "p1" above and still use  
> "path1" here?

No, changed.

> I might miss something, but I think
>  call UMASK(MASK=a, OLD=o)
> does not work as OLD= is still not recognized.

Changed, and fixed the doc (which only mentionned the subroutine, and  
not the function).

> -	     a, BT_CHARACTER, dc, REQUIRED);
> +	     "path", BT_CHARACTER, dc, REQUIRED);
>
> Shouldn't be there a constant above like it was done for the others?
> I don't mind "path" but it is a bit inconsistent.
>
>
> Ditto:
> -	      c, BT_INTEGER, 4, REQUIRED);
> +	      "seed", BT_INTEGER, 4, REQUIRED);
>
> Ditto:
> -	      c, BT_CHARACTER, dc, REQUIRED);
> +	      "string", BT_CHARACTER, dc, REQUIRED);
> (Doesn't there exist stg already?)o
>
> -	      c, BT_CHARACTER, dc, REQUIRED, st, BT_INTEGER, di, OPTIONAL);
> +	      "path", BT_CHARACTER, dc, REQUIRED, st, BT_INTEGER, di,  
> OPTIONAL);

I put the strings directly in there because they're only used once  
(and std is in add_subroutines(), not add_functions()).

I agree it's (a wee bit) inconsistent, but I don't think we should  
care so much. Actually, I think it would make it much easier to read  
and maintain if only literal strings were used (and no "const char *"  
variables. I can't believe it's much of a gain for the compiler anyway.

> -@code{DTIME(TARRAY, RESULT)}
> +@code{DTIME(VALUES, TIME)}
>
> I don't mind VALUES=, but I want to point out that both g77 and  
> ifort have TARRAY=
>
>
> Ditto ETIME, IDATE [such as g77; except that ifort has idate(i,j,k)  
> or idate(iarray)],
> LTIME.

I thought about that, and chose for consistency. People really  
shouldn't rely on argument names for vendor intrinsics; and as a  
matter of fact, they don't (as a Google codesearch shows on all these  
functions).


Attached is the updated version of the patch, again fully tested. OK  
to commit?

FX

Attachment: intrinsics2.ChangeLog
Description: Binary data

Attachment: intrinsics2.diff
Description: Binary data