On Mon, May 26, 2025 at 09:30:59AM +0000, Yuao Ma wrote:
> Hi Steve,
>
> > I looked at the patch in a bit more detail, and
> > I am not thrilled with large-scale whitespace
> > changes mingled with functional changes. It makes
> > the patch harder to read and review.
>
> I'm not sure which file you're referring to.
>
> If it's mathbuiltins.def, I'll need to add extra spaces to maintain argument
> alignment when I add the seven new built-ins.
>
> If it's intrinsic.cc, the issue is related to clang-format usage. You can find
> a more detailed explanation at
> https://gcc.gnu.org/pipermail/fortran/2025-May/062193.html.
>
It's intrinsic.cc.
/* Two-argument version of atand, equivalent to atan2d. */
- add_sym_2 ("atand", GFC_ISYM_ATAN2D, CLASS_ELEMENTAL, ACTUAL_YES,
- BT_REAL, dr, GFC_STD_F2023,
- gfc_check_atan2, gfc_simplify_atan2d, gfc_resolve_trigd2,
- y, BT_REAL, dr, REQUIRED,
- x, BT_REAL, dr, REQUIRED);
+ add_sym_2 ("atand", GFC_ISYM_ATAN2D, CLASS_ELEMENTAL, ACTUAL_YES, BT_REAL,
dr,
+ GFC_STD_F2023, gfc_check_atan2, gfc_simplify_atan2d,
+ gfc_resolve_trig2, y, BT_REAL, dr, REQUIRED, x, BT_REAL, dr,
+ REQUIRED);
What is the functional change in the above? It is somewhat difficult
to see a single character change. It's much easier to see (and detect
possible typos) in the following:
/* Two-argument version of atand, equivalent to atan2d. */
add_sym_2 ("atand", GFC_ISYM_ATAN2D, CLASS_ELEMENTAL, ACTUAL_YES,
BT_REAL, dr, GFC_STD_F2023,
- gfc_check_atan2, gfc_simplify_atan2d, gfc_resolve_trigd2,
+ gfc_check_atan2, gfc_simplify_atan2d, gfc_resolve_trig2,
y, BT_REAL, dr, REQUIRED,
x, BT_REAL, dr, REQUIRED);
To be clear, do not use clang-format if it inserts large-scale
whitespace changes. I'll also contend that that final result
in the latter is easier for a programmer to read. The 2nd line
is return type info and standard conformation. The 3rd line
contains the checking, simplification, and iresolve functions.
The remaining lines are the dummy arguments with one per line.
--
Steve
