[PATCH] D78424: [flang] Added Semantic Checks for 2 Data constraints and fixed the semantic errors in 3 test cases

[Peter Klausler via Phabricator via llvm-commits]

klausler requested changes to this revision.
klausler added inline comments.
This revision now requires changes to proceed.


================
Comment at: flang/lib/Semantics/check-data.cpp:62
+    if (!isRightmostComponent_) {
+      if (IsPointer(lastSymbol)) { // C877
+        symbolCheckStatus = false;
----------------
These state variables are still too confusing and they're going to lead to bugs later when people modify this code.

I think it would be more clear if there were a single state variable that allowed a pointer to appear, and it would be initialized to `true` if and only if the entire designator were a simple Symbol or a Component.  The state variable would be reset to `false` during the analysis of a Component.


================
Comment at: flang/lib/Semantics/check-data.cpp:100
+    } else {
+      return CheckAnySymbol(symbol);
+    }
----------------
This looks like you don't apply checks for dummy arguments, functions, blank COMMON, and so on if the designator is a component.  If so, that's wrong.  `DATA DUMMYARG%X / 1.0 /`.


================
Comment at: flang/lib/Semantics/tools.cpp:752
+bool IsAutomaticObject(const Symbol &symbol) {
+  if (symbol.IsDummy()) {
+    return false;
----------------
What about pointers and allocatables and functions and statement functions and construct entities and probably other types of symbol?  I think your function can return `true` for them, and that seems wrong if the symbol isn't a local object.


================
Comment at: flang/test/Semantics/data04.f90:42
+      !ERROR: Data object part 'b' must not be an automatic object
+      DATA b(0) /1/
+      f = i *1024
----------------
Please extend these tests to include cases where the dummy argument (or whatever) is not the whole designator; add subscripts, substring references, components, &c.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78424