Skip to content

Semantic checks for C702 #973

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

Merged
merged 1 commit into from
Feb 7, 2020
Merged

Semantic checks for C702 #973

merged 1 commit into from
Feb 7, 2020

Conversation

psteinfeld
Copy link
Collaborator

C702 (R701) A colon shall not be used as a type-param-value except in the
declaration of an entity that has the POINTER or ALLOCATABLE attribute.

I added code to the visitor for a TypeDeclarationStmt to check for the
'LEN' type parameter for strings and to loop over the type parameters
for derived types.

I also ran into a few situations where previous tests had erroneously
used a colon for type parameters without either the POINTER or
ALLOCATABLE attribute and fixed them up.

@@ -0,0 +1,45 @@
subroutine s1()
! C701 (R701) The type-param-value for a kind type parameter shall be a
! constant expression. This constraint looks like a mistake in the standard.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you think that C701 is a mistake? Kind type parameter values must be constant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question. I can't remember why, but when I first started looking at type parameters, I thought I saw an inconsistency and added this comment. I'll remove it.

@@ -3,20 +3,20 @@
! Note: Module files are encoded in UTF-8.

module m
character(kind=4,len=:), parameter :: c4 = 4_"Hi! 你好!"
character(kind=4,len=7), parameter :: c4 = 4_"Hi! 你好!"
Copy link
Collaborator

Choose a reason for hiding this comment

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

* would be better than explicit lengths here as a replacement for :.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, Peter. I'll change it.

@@ -1483,6 +1484,9 @@ Attrs AttrsVisitor::GetAttrs() {
CHECK(attrs_);
return *attrs_;
}
bool AttrsVisitor::HasPointerOrAllocatable() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this function improves upon the original predicate expression in terms of readability or isolating complexity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This surprises me. To be clear, there are three places in resolve-names.cpp where we test to see if the POINTER or ALLOCATABLE attribute is present. You're saying that it would be better to repeat the code in these three instances than to have a separate function?

Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the benefit of wrapping that simple predicate up in a function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It reduces the size of the code.

If you'd rather eliminate this function, I'm happy to do that. It seemed wrong to me to copy/paste existing code rather than sharing it. I'll remove the function.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Defining the function increases the number of lines of code by 3, yes? That's not a big deal, but it's surely not a reduction in code size. Is there some other benefit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not that I know of.

C702 (R701) A colon shall not be used as a type-param-value except in the
declaration of an entity that has the POINTER or ALLOCATABLE attribute.

I added code to the visitor for a TypeDeclarationStmt to check for the
'LEN' type parameter for strings and to loop over the type parameters
for derived types.

I also ran into a few situations where previous tests had erroneously
used a colon for type parameters without either the POINTER or
ALLOCATABLE attribute and fixed them up.
@psteinfeld psteinfeld merged commit 351a7d5 into master Feb 7, 2020
@psteinfeld psteinfeld deleted the ps-c702 branch February 7, 2020 18:28
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Apr 9, 2020
C702 (R701) A colon shall not be used as a type-param-value except in the
declaration of an entity that has the POINTER or ALLOCATABLE attribute.

I added code to the visitor for a TypeDeclarationStmt to check for the
'LEN' type parameter for strings and to loop over the type parameters
for derived types.

I also ran into a few situations where previous tests had erroneously
used a colon for type parameters without either the POINTER or
ALLOCATABLE attribute and fixed them up.

Original-commit: flang-compiler/f18@a1a95bf
Reviewed-on: flang-compiler/f18#973
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Apr 9, 2020
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this pull request Oct 7, 2022
C702 (R701) A colon shall not be used as a type-param-value except in the
declaration of an entity that has the POINTER or ALLOCATABLE attribute.

I added code to the visitor for a TypeDeclarationStmt to check for the
'LEN' type parameter for strings and to loop over the type parameters
for derived types.

I also ran into a few situations where previous tests had erroneously
used a colon for type parameters without either the POINTER or
ALLOCATABLE attribute and fixed them up.

Original-commit: flang-compiler/f18@a1a95bf
Reviewed-on: flang-compiler/f18#973
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