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

[Anchu Rajendran S via Phabricator via llvm-commits]

anchu-rajendran marked 4 inline comments as done.
anchu-rajendran added inline comments.


================
Comment at: flang/lib/Semantics/check-data.cpp:100
+    } else {
+      return CheckAnySymbol(symbol);
+    }
----------------
anchu-rajendran wrote:
> klausler wrote:
> > 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 /`.
> @klausler ,This is implemented in line 76 in the same file . I see that, (for the examples I could think of,) when a component is used, only the first symbol needs to be checked for checks in `CheckFirstSymbol`.  Line 116 in data04.f90 covers this case and emits proper error message as the checks are done on component. similarly lines 88, 119, 140 in the same file covers other cases  
> One thing that is lacking here is none of these tests are done on subscripts. However, I could not create a test case were subscript is a constant (enforced by C875) and would violate one of the other constraints.  (eg, C874 is not checked on subscripts). Can I know your opinion on this? 
The line numbers are changed in recent diff to 119, 122, 148, 154  in data04.f90 for `CheckFirstSymbol` and 85, 88, 91 for `CheckAnySymbol`


================
Comment at: flang/lib/Semantics/tools.cpp:752
+bool IsAutomaticObject(const Symbol &symbol) {
+  if (symbol.IsDummy()) {
+    return false;
----------------
anchu-rajendran wrote:
> klausler wrote:
> > 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.
> If it is a dummy, the function returns `false`. The only cases it returns true are (i) if length of a character is given by a non-constant expression (ii) the arrays with atleast one of the bounds is a non-constant expression. Else, the conservative value returned is `false`. Standard defines automatic object as 
> "nondummy data object with a type parameter or array bound that depends on the value of a specification-expr that is not a constant expression " 
> As kind parameter is always a constant expression, i did checks on only the length parameter.
> 
> For the cases you pointed out, I did check for pointers & allocatables and I saw it returns false. @klausler, Can you hint me an example code  that would make this not behave as expected?
Pointers and allocatables are checked and examples are added to data04.f90 at line numbers: 42, 45


================
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
----------------
anchu-rajendran wrote:
> klausler wrote:
> > Please extend these tests to include cases where the dummy argument (or whatever) is not the whole designator; add subscripts, substring references, components, &c.
> lines 88, 116, 119, 140 in the same file cover the cases
The line numbers are changed in recent diff to 119, 122, 148, 154  for `CheckFirstSymbol` and 85, 88, 91 for `CheckAnySymbol`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78424