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
Further semantics checks for procedure references #782
Conversation
checkpoint checkpoint
… intrinsics; further shape analysis of intrinsic results
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall check-calls.cc
looks great to me. Below are a few questions and maybe some small issues I saw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything builds and tests correctly for me. I don't understand most of this code, but it generally looks good.
It's not directly part of this pull request, but I noticed that lib/evaluate/shape.cc seems to have an unnecessary reference capture on line 398.
messages.Say("Target type '%s' is not compatible with '%s'"_err_en_US, | ||
that.type_.AsFortran(lenstr.str()), type_.AsFortran()); | ||
messages.Say( | ||
"%1$s type '%2$s' is not compatible with %3$s type '%4$s'"_err_en_US, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does $s
mean in the error format string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It selects the actual argument to be formatted. In this case, where 1$
through 4$
are used, the effect is the same as if they had not been present. I inserted them here in response to a comment about easing internationalization.
@@ -84,6 +84,10 @@ bool CanBeTypeBoundProc(const Symbol *); | |||
const Symbol *FindUltimateComponent( | |||
const DerivedTypeSpec &type, std::function<bool(const Symbol &)> predicate); | |||
|
|||
// Returns an immediate component of type that matches predicate, or nullptr. | |||
const Symbol *FindImmediateComponent( | |||
const DerivedTypeSpec &, const std::function<bool(const Symbol &)> &); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be helpful to define "immediate component" in the comment below that defined "ultimate component", etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do.
@@ -0,0 +1,475 @@ | |||
// Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git seems to be treating this as a new file rather than a move, so I can't tell if there are changes to review here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are, it's not just a move. I do not know why github failed to track the move on this file when it seemed to be able to do so on the other file that I moved (check-call.h
). I will mail you a diff.
checkpoint checkpoint Original-commit: flang-compiler/f18@99d12a7 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@7996c85 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@4b71f00 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@8c076bd Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@2691da3 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@9cc70dc Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@eb43df8 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
…HARACTER intrinsics; further shape analysis of intrinsic results Original-commit: flang-compiler/f18@561f596 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@b9c890c Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@43720b5 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@acd307c Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@e78db89 Reviewed-on: flang-compiler/f18#782
…/pmk-calls Further semantics checks for procedure references Original-commit: flang-compiler/f18@fbdd919 Reviewed-on: flang-compiler/f18#782
checkpoint checkpoint Original-commit: flang-compiler/f18@99d12a7 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@7996c85 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@4b71f00 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@8c076bd Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@2691da3 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@9cc70dc Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@eb43df8 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
…HARACTER intrinsics; further shape analysis of intrinsic results Original-commit: flang-compiler/f18@561f596 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@b9c890c Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@43720b5 Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@acd307c Reviewed-on: flang-compiler/f18#782 Tree-same-pre-rewrite: false
Original-commit: flang-compiler/f18@e78db89 Reviewed-on: flang-compiler/f18#782
These checks enable
call03.f90
to pass. Other changes in this PR deal with fall-out from enabling these checks on some of our correctness tests (some bugs were exposed, now fixed). The checking code itself has migrated over tolib/semantics
, which seems to be a more natural home. Intrinsic functions that returnCHARACTER
lengths or indices have been reverted to returning (by default)INTEGER
results of the default kind, rather than subscript-sized integers.