Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more rules on preprocessor expressions and their types #16

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gklimowicz
Copy link
Member

Change all preprocessor expressions for #if, #elif to evaluate to integer values. Define the relationship of Fortran logical constants and operators to their C counterparts.

Addresses #14.

Change all preprocessor expressions for #if, #elif
to evaluate to integer values. Define the relationship
of Fortran logical constants and operators to their
C counterparts.

Addresses #14.
@gklimowicz gklimowicz requested a review from bonachea February 1, 2025 03:15
Copy link
Collaborator

@bonachea bonachea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making a great start on this!

This is a non-trivial change and I have a correspondingly large amount of feedback. I may have even more feedback after further updates and further thought.

However I think we're on the right track here, and as I've already said I really want to see us adopt the core changes proposed in #14.

'true'.
10 .FALSE. is treated as the integer 0. .TRUE. is treated as the
integer 1.
11. Undefined identifiers are treated as zero, as in CPP.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor clarification, inspired by C spec wording:

Suggested change
11. Undefined identifiers are treated as zero, as in CPP.
11. Any undefined identifiers that remain after macro expansion (including those
lexically identical to keywords or intrinsics) are treated as zero, as in CPP.

Comment on lines +74 to +76
15. Integer expressions in preprocessor directives are calculated in
the maximum precision the processor supports. (There are no KIND
specifiers on integer constants in the preprocessor.)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it should be two separate rules. IIUC the parenthetical is actually prohibiting certain syntax, so it should not be parenthetical.

Comment on lines +69 to +71
13. The Fortran operators .AND., .OR., ,NOT., .EQ., .NE., .EQV., and
.NEQV. calculate the save values as the C operators &&, ||, !, ==,
!=, ==, and !=, respectively.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you're assuming we also accept the suggestion in issue #13 to add these additional Fortran operators to the draft. If we were to merge this PR alone as-is, these new rules would be inconsistent with the earlier list of operators.

This PR purports to address only #14 (which IMO has a strong stand-alone motivation), and I'd like to keep it focused on that proposal without conflating the orthogonal issue #13 (where the motivation may be seen as potentially weaker).

Please decouple these two PRs. I suggest removing the "new" operators above and getting this PR into a mergeable state to address solely #14, and then PR #15 can later introduce the obvious extension for the operators proposed by #13.

Suggested change
13. The Fortran operators .AND., .OR., ,NOT., .EQ., .NE., .EQV., and
.NEQV. calculate the save values as the C operators &&, ||, !, ==,
!=, ==, and !=, respectively.
13. The Fortran operators .AND., .OR., ,NOT., =, /= evaluate to the same values as
the C operators &&, ||, !, ==, and !=, respectively.

Comment on lines +72 to +73
14. The Fortran operators .LT., .LE., .GT., and .GE. calculate the
same values as the C operators <, <=, >, and >=, respectively.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here, please move this new rule to PR #15

Suggested change
14. The Fortran operators .LT., .LE., .GT., and .GE. calculate the
same values as the C operators <, <=, >, and >=, respectively.

Comment on lines +67 to +68
12. C character constants (such as 'A', '\n') are treated as
integer values, as they are in C.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This run-on list of rules needs to be restructured to clarify that the rule above (and possibly others?) only apply to the conditional expression in #if/#elif. For example this does not impact macro expansion in source.

Comment on lines +61 to +63
9. Expressions in #if and #elif directives evaluate to INTEGER values.
Zero values are treated as 'false'. Non-zero values are treated as
'true'.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When it comes time to actually write spec wording for this, it's not enough to say the expression "evaluate to INTEGER values". There must be additional constraints to rule out (for example) character strings in preprocessor conditional expressions.

For now it might be sufficient to say something like

Suggested change
9. Expressions in #if and #elif directives evaluate to INTEGER values.
Zero values are treated as 'false'. Non-zero values are treated as
'true'.
9. Expressions in #if and #elif directives must be integer constant expressions as in CPP
(with the extensions described below), and evaluate to INTEGER values.
As in CPP, zero values are treated as 'false'. Non-zero values are treated as
'true'.

Comment on lines +64 to +65
10 .FALSE. is treated as the integer 0. .TRUE. is treated as the
integer 1.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about .false. and .true.?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants