[flang-commits] [PATCH] D137204: [flang] Add check for constraints on event-stmts

[Katherine Rasmussen via Phabricator via flang-commits]

ktras added inline comments.


================
Comment at: flang/include/flang/Evaluate/tools.h:1202-1203
 bool IsBuiltinCPtr(const Symbol &);
+bool IsEventType(const DerivedTypeSpec *);
+bool IsLockType(const DerivedTypeSpec *);
 // Is this derived type TEAM_TYPE from module ISO_FORTRAN_ENV?
----------------
PeteSteinfeld wrote:
> ktras wrote:
> > PeteSteinfeld wrote:
> > > Is Evaluate/tools.h the right place for these functions?
> > > 
> > > Note that the function `IsOrContainsEventOrLockComponent()` is in Semantics/tools.h.  It also looks like all references to these functions come from within the Semantics directory.  Putting them all in the same files and directory makes more sense to me.
> > Thanks for the feedback. I put these new functions in `Evaluate/tools.h` since `IsTeamType` and `IsEventTypeOrLockType` are both there. I don't have a strong feeling where they should go, but if I move the new functions, then I think those other functions should probably also be moved. What do you think?
> Gee, `IsTeamType` is referenced in `Evaluate/intrinsics.cpp`, so it can't be moved even though it's logically related to the other functions.  But `IsEventTypeOrLockType` is only referenced from the `Semantics` directory.  For locality of reference, it make sense to me to move them.
What you said about `IsTeamType` reminded me, I will need to call `IsEventType` from `Evaluate/intrinsics.cpp` as well when I add `event_query` to the list of intrinsic subroutines. I will not need to call `IsLockType` from that file. What would you think in that case? Still move `IsEventType`, `IsLockType`, and `IsEventTypeOrLockType`? Or just the last two that don't/won't have references in the `Evaluate` directory?

@klausler It looks like you originally wrote `IsEventTypeOrLockType` and `IsTeamType`, do you have an opinion on this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137204/new/

https://reviews.llvm.org/D137204