[patch,doc] Fix intrinsic arguments names
FX
fxcoudert@gmail.com
Sat May 16 22:40:00 GMT 2009
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: intrinsics2.ChangeLog
Type: application/octet-stream
Size: 430 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20090516/2e9cac16/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: intrinsics2.diff
Type: application/octet-stream
Size: 36422 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20090516/2e9cac16/attachment-0001.obj>
More information about the Gcc-patches
mailing list