Hi,
Yuao Ma wrote:
I'm pretty swamped for the next couple of days
Same issue here - hence, I haven't completed the review ...
You're absolutely right that the best way to keep changes minimal is to just
rename the `*resolve*` function.
I missed the
-gfc_resolve_trigd,
+gfc_resolve_trig,
change. This brings me to the following regarding the changelog entry:
* intrinsic.cc (add_functions): Register new intrinsics.
This does not imply that change and it the change is small enough
that it can easily to be missed when the line breaks change.
Regarding:
* intrinsic.h (gfc_simplify_acospi): New.
(gfc_simplify_asinpi): New.
(gfc_simplify_atanpi): New.
(gfc_simplify_atan2pi): New.
...
You can save space by using:
* intrinsic.h (gfc_simplify_acospi, gfc_simplify_asinpi,
gfc_simplify_atanpi, gfc_simplify_atan2pi): New.
(I left out some more 'new' but you get the pattern. Inside the same
file, combining is possible.)
Regarding:
* iresolve.cc (gfc_resolve_trigd): Rename. This does not really help. I
think generally, we use something like: * value-range.cc
(irange_bitmask::irange_bitmask): Rename from
get_bitmask_from_range and tweak.
(i.e. state the old name in the description) or use
(riscv_ext_flag_table): Rename to ...
(riscv_extra_ext_flag_table): this, and drop most of definitions to make
clear what changed. * * *
The challenge is I used clang-format (with the
`contrib/clang-format` file) for formatting. I know it's just a
recommendation,
but it's super helpful for keeping the code style consistent, and
doing that
manually is a pain.
Indeed a challenge. – While I'd prefer to have only the 'd' change, I guess
I am also fine with using clang-format …
But this change should be should also mentioned in the ChangeLog to
avoid going through the change letter by letter to understand whether
there was a significant change or it was just reformatted.
Tobias
