Hi Thomas, >>>> and in that >>>> case I would argue that, beyond being "not useful", it's even illegal, >>>> so why not throw a hard error, if we can infer at compile-time that >>>> the target is non-contiguous? >>> >>> Problem is, we cannot infer this from the tests done. >>> We would also have to add a test if the array is empty >>> or that it contains only a single element, and that (I think) >>> is a) impossible in the general case, and b) not worth the bother. >> >> I'm not sure I understand which cases you're worried about here. Maybe >> you can give an example? > > real, dimension(5,5), target :: a > real, dimension(:,:), pointer, contiguous :: ap > ap => a(4::2,4::2) > > points to a single element, which is (by definition) contiguous.
yes, I see that your current patch mishandles this case. But why should it be impossible to detect that there is only a single element? If the triplet and the array bounds are constant, it is certainly possible. >> In any case, I think your test case is a bit short, so I extended it >> somewhat (see attachment) and found two cases along the way where your >> patch throws a warning but shouldn't: >> >> r => x(::-1) > > This one is _not_ contiguous; contiguos implies stride==1. Ok, apparently I did not read the F08 standard carefully. I guess the mention of the "array element order" in chapter 5.3.7 forbids the negative stride (which means reversed order). >> Apart from the two mishandled cases above, one other thing comes to my >> mind: It might be a good idea to apply your checks not only to pointer >> assignments, but also to dummy arguments (passing a non-contiguous >> array to a contiguous dummy pointer), where the same rules should >> apply. > > This is true. > > There are also a few other checks that I missed, for > example > > subroutine foo(a) > real, dimension(:), target, intent(inout) :: a > real, dimension(:), contiguous :: ap > ap => a > > I will also address this in a future version of the patch. How do you want to do that? I don't see a way to tell at compile time if 'a' here is contiguous (like in many other cases). I guess one would ultimately need a runtime check. Or do you want to warn unconditionally because it 'might' be non-contiguous? Cheers, Janus 2017-08-29 20:13 GMT+02:00 Thomas Koenig <tkoe...@netcologne.de>: > Hi Janus, > >>>>> I think an unconditional warning is OK >>>>> in this case because >>>>> >>>>> - Assigning to a pointer from an obvious non-contiguous target >>>>> is not useful at all, that I can see >>>> >>>> >>>> >>>> I guess you're talking about a *contiguous* pointer here, >>> >>> >>> Correct. >>> >>>> and in that >>>> case I would argue that, beyond being "not useful", it's even illegal, >>>> so why not throw a hard error, if we can infer at compile-time that >>>> the target is non-contiguous? >>> >>> >>> Problem is, we cannot infer this from the tests done. >>> We would also have to add a test if the array is empty >>> or that it contains only a single element, and that (I think) >>> is a) impossible in the general case, and b) not worth the bother. >> >> >> I'm not sure I understand which cases you're worried about here. Maybe >> you can give an example? > > > > real, dimension(5,5), target :: a > real, dimension(:,:), pointer, contiguous :: ap > ap => a(4::2,4::2) > > points to a single element, which is (by definition) contiguous. > >> In any case, I think your test case is a bit short, so I extended it >> somewhat (see attachment) and found two cases along the way where your >> patch throws a warning but shouldn't: >> >> r => x(::-1) > > > This one is _not_ contiguous; contiguos implies stride==1. > >> r => x2(2:3,1) > > > Correct, I will correct that one. > >> Apart from the two mishandled cases above, one other thing comes to my >> mind: It might be a good idea to apply your checks not only to pointer >> assignments, but also to dummy arguments (passing a non-contiguous >> array to a contiguous dummy pointer), where the same rules should >> apply. > > > This is true. > > There are also a few other checks that I missed, for > example > > subroutine foo(a) > real, dimension(:), target, intent(inout) :: a > real, dimension(:), contiguous :: ap > ap => a > > I will also address this in a future version of the patch. This will > take a bit of time, though. > > Regards > > Thomas
