- C876, C877 checks are implemented.
- Modified pre-fir-tree02.f90, block-data01.f90 and data01.f90 as they were not conforming to C876
- Fixed IsConstantExpr to check C879
Thank you for your review comments! I have made the suggested modifications.
The code has been changed to check all symbols so the related semantic errors with derived types can be caught.
Thank you for your suggestions! I have added extra code for handling derived types and tests with derived types for checking constraints related to automatic arrays, use association, allocatable variables and common blocks.
Thanks for addressing the comments.
Also check the following.
type t1 integer, allocatable :: arr(:) end type type(t1) d1 data d1/t1((/0,1/))/
Besides the constraints for the data statement, there are several other points mentioned in the data statement section. Like point 8 in 8.6.7 (test below) which says that the number of variables and data-statement-constants shall be the same. Are these outside the scope of your work? If so, please create a ticket somewhere so that we can come back to it later.
program mn character(len=3) :: n1, n2 data n1, n2 / 'f18' / end
I wasn't sure whether using the type from the module makes the variable d1 a use associated variable. What is your conclusion?
Fixed a few corner cases:
Implemented a function: GetFirstName
module dt type d real r end type end module program test use dt type(d) :: d1 data d1%r/2.5/ end program
In the above example, d1%r should not be identified as use-associated since, d1 is declared in the same scope. Previous revision identifies d1%r as use associated since the semantic checks were performed on last name r and not on first name:d1. The current revision applies checks on first name.
A few other corner cases will also be caught with this modification. An example follows:
module m2 type newType integer number end type contains subroutine checkDerivedType(m2_number) type(newType) m2_number DATA m2_number%number /1/ end end
In the above example, m2_number%number should not be used as a data obect as it is a dummy argument. This constraint will only be checked if check is applied on first name.
The current revision does not catch this error. I dont think this case is covered under the Constraint C876 (or C877)
C876 says a variable whose designator appears as a data-stmt-object shall not be an allocatable variable . This means d1 cannot be an allocatable (and it does not say anything about the fields of d1). In this case, d1 is not.
Earlier, Anchu said that she would change this code to use typed expressions. Has this been done? It doesn't look like it. It took me most of the week to rework her earlier patch to use the right representation. None of these analyses should be done on the parse tree, and the new functions in parser/tools.h aren't necessary.
Every serious production Fortran compiler allows DATA initialization of COMMON in any subprogram;. Ignore the standard here, or change the error into a warning message.
Thank You! I went through your patch and now, I get your point on why we should implement semantic checks on typed expressions. I have tried to implement C876 and C877 on top of this patch and I could perform those checks on typed expressions. I will submit those for review after you merge your patch.
The change to convert code to have checks on typed expressions was asked by you on my previous patch (https://reviews.llvm.org/D78008). I asked for a merge as I was not sure about the changes you were expecting and I thought the code in this patch would be least affected. Sorry for my misunderstanding and thanks for the changes!
The logic here is wrong. Just recurse on the base (return (*this)(component.base());). If the base is an array element reference with non-constant subscripts, the component reference isn't a constant.
This code doesn't implement the C877 constraint correctly. A pointer is allowed in a DATA statement object only if it is the entire rightmost part-ref. Your code would accept subscripts or a substring reference that was applied to a pointer component (A%PTR(1)(2:3)).
This member function doesn't use any state from the class; it could be made static, but it would be even better if it were in lib/Semantics/tools.cpp as it will probably be useful in other cases.
Please follow the naming conventions.
This code would return true for an explicit-shape dummy argument array with non-constant bounds, yes? Such objects are not "automatic". I think that you need a better name for this predicate.
- Addressed the review comments
- Modified IsAutomaticArray to check all automatic objects as described in 3.11
- Added extra checks to emit error if right most pointer reference in a component is subscripted (C877)
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.
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 /.
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.
Please extend these tests to include cases where the dummy argument (or whatever) is not the whole designator; add subscripts, substring references, components, &c.
@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
I have returned false than reporting the errors. Thank you!
Subscripts can have any kind of DataRef. If a subscript is a%b%c(i), i think it is better to have tests on this DataRef separately so functions like CheckFirstSymbol will be called on a and say if a is a dummy argument, it reports an error.
For eg., consider a%b(c%d) is the data object being analyzed, where b is an array. In this case, when execution reaches checks on subscript, this.isFirstSymbolChecked_ will be true and this.hasSubscript_ will be true. For any checks that need to be done on subscript , I think it is better to set these variables for each subscript separately as it can mark the beginning of a DataRef. However I cannot find an use case where this checker will be used to report more errors (as the subscript needs to be a constant). I shall remove this part of code if it not expected to catch more semantic errors.
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
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?
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 in data04.f90 for CheckFirstSymbol and 85, 88, 91 for CheckAnySymbol
Pointers and allocatables are checked and examples are added to data04.f90 at line numbers: 42, 45
The line numbers are changed in recent diff to 119, 122, 148, 154 for CheckFirstSymbol and 85, 88, 91 for CheckAnySymbol
I'm nearly done with the implementation of the rest of semantic processing for DATA statements (iterating through the objects and values, putting them into correspondence, and constructing a single initializer for each object).
You might as well commit your changes, and I'll clean up any remaining problems here when I merge your changes with mine.