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

[sameeran joshi via Phabricator via llvm-commits]

sameeranjoshi marked 2 inline comments as done.
sameeranjoshi added inline comments.


================
Comment at: flang/lib/Semantics/check-select-type.cpp:139
+  }
+  struct TypeCase {
+    explicit TypeCase(const parser::Statement<parser::TypeGuardStmt> &s,
----------------
klausler wrote:
> sameeranjoshi wrote:
> > klausler wrote:
> > > Is `struct TypeCase` nested in `class TypeCaseValues`?
> > Yes, the similar way select-case has `struct Case` inside `class CaseValues`.
> But why is this necessary?
Here's how I see it.
`TypeCase` nested in `TypeCaseValues` creates a logical nesting which becomes easy to read code and figure out the similar functions which would operate on some particular data of struct.

The way `select-type-construct` in below snippet from standard 

```
R1152 select-type-construct 
select-type-stmt
[ type-guard-stmt block ] ...
[ type-guard-stmt block ] ...
...
end-select-type-stmt
```

Tries to seperate `type-guard-stmt block` and created a list of such blocks, similarly I see for `TypeCase` .
Also various helper functions and data members which logically would come in mind when it comes to implementing  `type-guard-stmt block`  are grouped in same struct.

Do you suggest instead of having a nested struct better to have these functions and struct data members in the parent class?


================
Comment at: flang/lib/Semantics/check-select-type.cpp:134
+    }
+    // TODO: C1162
+    return true;
----------------
klausler wrote:
> C1162 is important; please implement it.  There can't be TYPE IS or CLASS IS cases in a SELECT TYPE that are not possible for the selector.
I have a test already added in selecttype01.f90 in subroutine CheckC1162.
If @PeteSteinfeld  @klausler that LG, I think `IsExtensibleType` when passed with `parentDerived` would help in code implementation?

Can we extract the ancestor chain of a particular type as well as the types which derive from it?

Can you point some test file if present or the code file which could be already handling this?



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

https://reviews.llvm.org/D79851