chapel-lang / chapel

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

Using `bool` as a vararg count causes internal error #20371

Open DanilaFe opened 2 years ago

DanilaFe commented 2 years ago

Summary of Problem

I get the following compiler error:

bug.chpl:1: internal error: RES-EXP-RGS-0271 chpl version 1.28.0 pre-release (0)

Internal errors indicate a bug in the Chapel compiler ("It's us, not you"),
and we're sorry for the hassle.  We would appreciate your reporting this bug --
please see https://chapel-lang.org/bugs.html for instructions.  In the meantime,
the filename + line number above may be useful in working around the issue.

I expected to no get an internal error, though exactly how Chapel should behave with boolean vararg counts is something I cannot say. I was actually checking the compiler as a reference.

Steps to Reproduce

proc test(param n: bool, args ... n) {
    writeln(args);
}
test(true, true);

Compile command:

chpl bug.chpl

Configuration Information

chpl version 1.28.0 pre-release (0)
  built with LLVM version 14.0.6
Copyright 2020-2022 Hewlett Packard Enterprise Development LP
Copyright 2004-2019 Cray Inc.
(See LICENSE file for more details)

I'm using a from-source build while M1 homebrew builds are malfunctioning, but I'm pretty sure this happened on 1.26.0_1.

bradcray commented 2 years ago

Trying to think about whether I would expect this to work / be supported or not: Do we support 0 as a varargs count currently? My recollection is that varargs don't currently match against 0 args (e.g., https://github.com/chapel-lang/chapel/issues/9373) where a big part of that relates to not supporting 0-tuples currently since varargs and tuples are closely related in Chapel.

All that said, this issue is still valid (users shouldn't get internal errors), just pointing out that there may be more to it than just the bool (?).

DanilaFe commented 2 years ago

Do we support 0 as a varargs count currently?

No, we do not. However, my intuition is that if booleans were allowed for counts, then true would be equivalent to 1, so the above code should work and print true.

benharsh commented 2 years ago

We do not support zero-varargs at this time. I believe this would be much easier to implement in dyno, but might require some design discussion...

I just want to point out briefly that dyno current handles this param-bool case differently. In the following program we currently decide to not resolve the call (for now) rather than coercing to an integer:

proc fn(param n : bool, args...n) {
  var ret : args.type;
  return ret;
}
fn(true, true);

Long-term, I think we should make it an error if the count-expression is not a param integral since it is unlike that the user intended for that to happen. It would be trivial and much clearer for the user to cast to an int: args...n:int.

bradcray commented 2 years ago

However, my intuition is that if booleans were allowed for counts, then true would be equivalent to 1, so the above code should work and print true.

I agree with that. But I think if the only valid boolean count that can be given is true, then it makes the argument for supporting the feature (rather than just making it an error) weaker.

OTOH, I am a fan of supporting 0-tuples and 0-match varargs (as are vocal users), so really what I'd like to see us do is add that support and then support both true and false cases.

All that said, until that day, I'd probably just make bool counts generate a user error (saying they're not supported) rather than working to add support for it but only permitting it to work in the true case (i.e., @benharsh's long-term vision, though I'd call that the thing we should do in the short-term).