On Wednesday 03 August 2011 17:55:00 Tobias Burnus wrote: > > Though variables in the general case can be components, I don't think it > > is the case here as only named variables are involved here. > > Does that sound right? > > The first part of the sentence sounds wrong: A component itself is not a > variable. I think you mean a "structure-component" - and for "var%comp" > both "var%comp" and "var" are variables Yes, OK
> and I belief both "var%comp" and > "var" are named. My reading is that a named variable is a variable that is an object-name. But either is fine w.r.t. your patch. > > "R602 variable is designator or expr" > "R601 designator is object-name or ... or structure-component" > "R613 structure-component is data-ref" > "R611 data-ref is part-ref [ % part-ref ]" > "R612 part-ref is part-name [ ( section-subscript-list ) ] [ > image-selector ]" > [...] > > The question is: Should we bother? Is there some genuine interest in > supporting valid but unusable type declarations? OK, I'm starting to understand the patch. > The problem is that > diagnosing the problem can get rather difficult. For instance: > > type t > type(lock_type) :: C > end type > type t2 > type(t), allocatable :: B > end type t2 > type t3 > type(t2) :: D > end type t3 > > is valid - however, it is invalid to use: > type(t) :: x[*], y > > However, how to write then the error message? "Error: Invalid > declaration at (1) as constraint C642 is violated" is probably not very > helpful, but should one really re-resolve the derived type? I would write: Variable at <y> shall be a coarray/have a codimension attribute as it has a non-coarray subcomponent of type LOCK_TYPE at <c> Best would be to have the full reference y%d%b%c in the error message. > There are > also many possible solutions: > > "B" should be a coarray, "C" could be also allocatable and a coarray, or > "B" could be not allocatable and "D" or "y" could be a coarray. > > Thus, writing a nice and helpful message gets pretty complicated. Thus, we should stick closely to the standard, point exactly what is prohibited, and not bother too much trying to provide some hints to the users. ;-) > And, as written, I do not see a compelling reason for not diagnosing it at > type-declaration time - even if the type is formally correct. Well, OK, but we should precise exactly why we reject it then. > > > Regarding the check itself - if one assumes that one wants to have an > error, I believe that part is correct. If there is a pointer, it cannot > be valid. Example 1: > > type t > type(lock_type), pointer :: lock1 > end type t > > I cannot write "lock1[:]" as in components, only allocatables are allowed: > > "C442 (R436) If a coarray-spec appears, it shall be a > deferred-coshape-spec-list and the component shall have the ALLOCATABLE > attribute." > > Turning the variable into a coarray as in > type(t) :: x[*] > does not help: the x%lock1 is not a coarray. "A subobject of a coarray > is a coarray if it does not have any cosubscripts, vector subscripts, > allocatable component selection, or pointer component selection." (Sect. > 2.4.7) > > On the other hand, if "type t" contains a noncoarray lock_type, one > cannot do use "type(t), pointer :: ptrcomp" as "...%ptrcomp%lock" > wouldn't be a coarray (cf. above) - and if "lock" in "...%ptrcomp%lock" > were a coarray, it would be invalid as: "C444 A data component whose > type has a coarray ultimate component shall be a nonpointer > nonallocatable scalar and shall not be a coarray." OK, it is starting to make sense now. I'm not very fond of it, but if you want to keep this diagnostic, at the very least put all that information in a comment. Best would be to provide it (or some of it) in the error message too. Currently there is a comment indicating that we check C1302. Fine. One looks at C1302: OK, if a component is like that, that constraint on the variable. Fine. The error's on variables, and there is neither allocatable nor pointer crazy stuff. One looks at the code then: if (pointer) error ("Component blah pointer blah") if (allocatable) error ("Component blah allocatable blah") What the fuck? Back to the standard then, is it a typo? Check C3102, C1320, C1032. Nothing... Err? So, please make it explicit why you reject pointer, etc... > > > Having said that, I just realized that the following program is not > rejected but it should: That's exactly the reason why I don't like it. It's sufficiently difficult to get it right while sticking closely to the standard that one doesn't want to try picking one rule every 40 pages and see what is left after intersecting them. [...] > Here, something similar applies: > > type t > integer, allocatable :: caf_comp[:] > type(lock_type) :: lock > end type t > type(t) :: x[*] > > It is invalid to make "x" a coarray as "t" already has coarray > components - but if I don't make "x" a coarray, x%lock is not a coarray, > which is invalid. Again, make it explicit in the code/in the error message. > > Thus, I believe there is no way that this error is printed for a type > declaration which can be used to create a valid named variable. > > >> > + || (!gfc_is_coarray (code->expr1) && !gfc_is_coindexed (code- > >> > > >> >expr1))) > > > > This part is quite OK. > > I have the feeling that !gfc_is_coindexed is superfluous once one has > > !gfc_is_coarray. > > I'm also not sure that the removal of `coarray' in the error message is > > needed as C1302 forces LOCK_TYPE entities to be coarrays or subobjects of > > coarrays. > > The LOCK_TYPE named object needs to be a coarray - the entity you pass > to LOCK does not have to be a coarray. Example: > > type(lock_type), type :: lock[*] > LOCK(lock) ! (1) > LOCK(lock[1]) ! (2) > > In (1), the argument is a scalar coarray of lock_type. But in (2) one > does not have a coarray but just a coindexed scalar of lock_type. OK. > > As "lock[1]" is not a coarray, I believe removing "coarray" from the > error message makes sense as does checking for both gfc_is_coarray and > gfc_is_coindexed. OK. > > > Looks like it's not sufficient. > > One can have a non-coarray LOCK_TYPE component and an other component > > that is a coarray. > > That's the reason for all the checks in parse.c. I make use of the fact > that one cannot have more than one coarray - if a component is a > coarray, the variable cannot be a coarray and vice versa. I think I have > covered all cases there, but I might be wrong (as I already have been > for the pointer, cf. above). I'll review the revised patch later today. With the information you have provided it should be better accepted. :-) Mikael