[PATCH] D79851: [Flang] Semantics for SELECT TYPE

[Pete Steinfeld via Phabricator via llvm-commits]

PeteSteinfeld added inline comments.


================
Comment at: flang/lib/Semantics/resolve-names.cpp:5101-5102
+
+    } else if (evaluate::HasVectorSubscript(
+                   *association.selector.expr)) { // C1158
+      Say(association.selector.source,
----------------
PeteSteinfeld wrote:
> sameeranjoshi wrote:
> > PeteSteinfeld wrote:
> > > C1158 also applies to selectors that are not variables.  Can you please add a check and test for that situation?
> > I was confused from the wordings of standard.
> > This is what I understand,
> > If selector is not a variable => throw respective error
> > if selector is a variable that has a vector subscript -> throw error
> > 
> > Is this what you meant, that I am missing the first check?
> Here's my understanding of C1158 in pseudo code:
>  
> ```
>  if (!selector-is-variable || selector-is-variable-with-vector-subscript) {
>     if (associate-name-appears-in-variable-definition-context ||
>       associate-name-subobject-appears-in-variable-definition-context) {
>       emit error message;
>     }
>   }
> ```
> 
> Suppose you have a function that returns a polymorphic type.  Then, you call that function in a select-type-stmt and associate it with an associate-name.  Then, suppose the associate-name appears on the left-hand side of an assignment statement or you use it as an actual argument to a procedure that has a dummy argument with INTENT(OUT) or INTENT(INOUT).  I think that's the situation that C1158 is trying to avoid.
> 
> So it's OK to use something other than a variable, as long as you don't try to change the value of the associate-name.  In standards-speak, that means that the associate-name can't appear in a "variable definition context" (see section 19.6.7 for details).  Note also that if the selector **is ** a variable, it's OK to use it in a variable definition context.
> 
> In fact, now that I've explained this in more detail.  I don't think that your existing check or test is correct.  To trigger an error, you should have a test where the associate-name appears on the left hand side of an assignment statement in the code of the construct of the SELECT TYPE statement.  The test should look something like this:
> 
> 
> ```
>   select type(func_call(x) => func_result)
>   type is (xxx)
>     call sub_with_inout_param(func_result) ! Bad since func-result is in a variable definition context
>   end select
> ```
> So to implement this check, you'll need to look at all occurrences of the associate-name in the body of the select-type construct and see if the associate-name is associated with something other than a variable **and** if it is used in a variable definition context.  In such cases, you should emit an error message.
> 
> As a model for implementation, you can look in in check-do-forall.cpp.  Constraint C1130 requires that a variable used in a DO CONCURRENT construct must have its locality explicitly specified.  So there's code in there that walks the DO CONCURRENT construct looking for variables that violate this requirement.
> 
> You might also find it useful to call the function WhyNotModifiable() in tools.h to figure out whether a Symbol is modifiable in a Scope.  See section 19.6.7 for details on variable definition contexts.
> 
I've been looking at the existing code in the compiler.  All that you may need to do is add some code to `WhyNotModifiable()` to handle SELECT TYPE associate-names.  I see that there's already a test for a name that's construct associated.  Start by writing tests that cover the two situations for C1158 and see if the appropriate things happen.


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

https://reviews.llvm.org/D79851