chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.77k stars 417 forks source link

Request: default forall intent on record with forwarded array #13276

Open BryantLam opened 5 years ago

BryantLam commented 5 years ago

General question. Feature Request.

This code tries to use forall on a record with a forwarded array field. Regular arrays work because of default intents, but the record has a const ref default intent that prevents the forwarded array from being modifiable in a forall. Should this code or something like it work?

The forwarded array is only a specific use case.

record R {
  forwarding var data: [1..8] int;
}

proc main() {
  var arr: [1..8] int;

  forall i in arr.domain {
     arr[i] = i;
  }

  var r: R;

  forall i in r.domain {
    r[i] = i; // error: illegal lvalue in assignment
  }

  forall i in r.domain {
    r.data[i] = i; // error: illegal lvalue in assignment
  }
}

chpl version 1.20.0 pre-release

BryantLam commented 5 years ago

Specification currently says:

Default Intent for Arrays and Record 'this' The default intent for arrays and for a this argument of record type (see §14.2) is ref or const ref. It is ref if the formal argument is modified inside the function, otherwise it is const ref.

I get the impression that this code should work.

Edit: Spoke too soon. This isn't a function. The Abstract Intents table shows that the default intent for a record is always const ref.

bradcray commented 5 years ago

I think that this is as currently expected. There have been discussions about giving record authors the opportunity to define a different default intent for their record types, but no firm proposals here yet.

This is probably obvious to Bryant, but for others who might come across this issue, task intents can be used to override the current const ref default and modify the record:

  forall i in r.domain with (ref r) {
    r[i] = i;
  }

  forall i in r.domain with (ref r) {
    r.data[i] = i;
  }
mppf commented 5 years ago

I think the question is answered. @BryantLam - would you like to close the issue, or change it into a feature request?

BryantLam commented 5 years ago

I changed this to a feature request. Not a priority to me at the moment. Thanks!

bradcray commented 5 years ago

Just to make sure I'm understanding: Is the request here what I was mentioning above: As a record author, to have the ability to explicitly request a different default intent for a type than the one the compiler / language would choose on its own?

BryantLam commented 5 years ago

Correct. I would want a feature to change the default intent of a record/class to an author-specified intent (flexible) or inherit the default intent of a different object/type/field (more restrictive, but applies specifically for this use case).

The underlying problem is that there's no [apparent] way to encapsulate (forward to) an array. How does the standard/Lists.chpl (list with parSafe) get around this issue to be usable with forall?

bradcray commented 5 years ago

Correct. I would want a feature to change the default intent of a record/class to an author-specified intent (flexible)

OK, thanks. I've opened up issue #13319 to try and capture this longstanding idea a bit more clearly (I thought we had an issue for it, but couldn't find it), and would be inclined to close this one as a duplicate of it if that would be OK with you (referencing this issue there before doing so of course).

or inherit the default intent of a different object/type/field (more restrictive, but applies specifically for this use case).

That's an interesting variation, but I'm not sure that it offers significant benefits over the more general form and feels potentially a little artificially limiting (e.g., If you're creating an int-like record, but it doesn't have an int-like field, you're stuck).

I don't know the answer w.r.t. the List type. @dlongnecke-cray / @mppf?

BryantLam commented 5 years ago

That's an interesting variation, but I'm not sure that it offers significant benefits over the more general form and feels potentially a little artificially limiting (e.g., If you're creating an int-like record, but it doesn't have an int-like field, you're stuck).

The variation is more of a consideration for users that don't want to know what the underlying intent is for a field (because they don't care or because of abstractions) but still want to set their record/class to that field's intent. E.g.,

record B {} // or possibly an array, or an exposed item by a library.
class C {
  forwarding var data: B;
}

As an author of C, I don't have to care what the intent of B is, even if it later changes.

bradcray commented 5 years ago

As an author of C, I don't have to care what the intent of B is, even if it later changes.

Oh, in this variation, were you proposing that the intent would be taken from the forwarding declaration, if one existed? What would you do for types that had multiple forwarding fields or multiple fields in addition to the forwarding ones?

BryantLam commented 5 years ago

I don't know what to do with multiple forwarding fields which is why I think the solution has to be something like the variation I suggested: the user should indicate which field's default intent they want to inherit/encapsulate toward/represent as. That said, I don't have a clear use case for multiple forwardings so I don't know how it impacts the overall use of a record like that.