chapel-lang / chapel

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

Saving nothing type in field with a declared type doesn't generate a helpful error message #20175

Open lydia-duncan opened 2 years ago

lydia-duncan commented 2 years ago

Summary of Problem

I tried making a field that stores a first class function use the nothing type and the compiler was unhappy. It outputs the following:

saveInField-nothing.chpl:10: In initializer:
saveInField-nothing.chpl:11: error: could not find a copy initializer ('init=') for type 'shared chpl__fcf_type_int64_t_chpl_bool' from type 'nothing'
$CHPL_HOME/modules/internal/SharedObject.chpl:255: note: this candidate did not match: _shared.init=(pragma"nil from arg"in take: owned)
saveInField-nothing.chpl:11: note: because actual argument #1 with type 'nothing'
$CHPL_HOME/modules/internal/SharedObject.chpl:255: note: is passed to formal 'in take: owned'
saveInField-nothing.chpl:11: note: other candidates are:
$CHPL_HOME/modules/internal/SharedObject.chpl:281: note:   _shared.init=(pragma"nil from arg"const ref src: _shared)
$CHPL_HOME/modules/internal/SharedObject.chpl:303: note:   _shared.init=(src: borrowed)
note: and 2 other candidates, use --print-all-candidates to see them

Edit: this error message was unhelpful, but it turns out I should have declared the field type when trying to do this. I suspect this is a more general problem than fcfs.

The details block has the actual scenario that motivated me to find this, for the curious:

I was playing around with storing a hasher function in my attempt at the DistributedMap implementation (so that users can have control over which locale the key hashes to). Because the key and value type fields make any hashing of them generic, I can't write a default function to use that lives outside of the DistributedMap type, and I can't make a field that stores a fcf of a method, because the field initialization doesn't know about the methods on the type yet. So what I was hoping to do was have the hasher field be set to `none` when it is relying on the default hasher function and just call a method I've defined instead. That ran into this problem, which I've boiled down into a simpler test case.

Steps to Reproduce

Source Code:

record Foo {
  var funcStored = false;
  var funcField: func(int, bool) = none;

  proc init(x: func(int, bool)) {
    funcStored = true;
    funcField = x;
  }

  proc init() {
    this.complete();
  }

  proc callTheField(arg: int) {
    if (funcStored) {
      if (funcField(arg)) {
        writeln("fcf with arg ", arg, " successful");
      } else {
        writeln("fcf with arg ", arg, " unsuccessful");
      }
    } else {
      writeln("no fcf stored");
    }
  }
}

proc bar(x: int): bool {
  if (x * 3 > 15) {
    return true;
  } else {
    return false;
  }
}

var f1 = new Foo(bar); // field is a fcf
f1.callTheField(2);
f1.callTheField(6);
var f2 = new Foo();  // field is none
f2.callTheField(17);

Compile command: chpl foo.chpl

Execution command: N/A

Associated Future Test(s): test/functions/firstClassFns/saveInField-nothing.chpl #20176

Configuration Information

bradcray commented 2 years ago

To me, this feels more like an "error message" issue than a "bug" issue in that only variables/fields of nothing type can store none values in Chapel, so it seems appropriate that you couldn't store none into that field. That said, the error message doesn't make this very clear, which is why I'd expect the issue to be to improve the error message.

mppf commented 2 years ago

I agree. I think instead of

record Foo {
  var funcStored = false;
  var funcField: func(int, bool) = none;
  ...
}

you would need to write

record Foo {
  type funcType; // an FCF type or 'nothing'
  var funcField: funcType;

  proc init(x: func(int, bool)) {
    funcType = func(int, bool);
    funcField = x;
  }

  proc init() {
    funcType = nothing;
    funcField = none;
  }

  ...
}
lydia-duncan commented 2 years ago

Ah, I see my confusion. I suspect part of this comes from a misunderstanding of the nothing type/none value due to not using it extensively myself and purely hearing its description. My understanding was that it would allow a field to be optimized out but that its original type was important and something that should be specified somewhere, and the obvious place to me was on the field itself. I'll update the test I filed with this information and play around with it more, thanks!

Thinking about this from a documentation perspective, I think this argues that initializers should be part of a (or at least this) type's documentation, so that a user of the type can know exactly what interface the fcf is expected to have. I also wonder if it would have helped me to have a commented out code example in the nothing primer that explicitly called out you can't give a field an explicit type if you wish it to be able to have none as a value.