Chapel: default comparison for records with default types is not symmetrical

Created on 23 Aug 2018  路  11Comments  路  Source: chapel-lang/chapel

record R {
  type t = int;
  var x:t;
}

proc main() {
  var r64 = new R(int(64), 64);
  var r32 = new R(int(32), 32);

  // This works on master
  writeln(r32 == r64);

  // this does not
  writeln(r64 == r32);
} 
Compiler Language

Most helpful comment

Speaking about assignments, what is currently implemented is:

  • we can't assign from age to SAT
  • we can't assign from r64 to r32
  • we CAN assign r64 = r32 in Michael's original example

I suggest that the last one should be fixed to be disallowed.

All 11 comments

A related question: should the r32 == r64 in the above fail to resolve in both cases?

PR #10873 will make the above test compile and run (and the behavior symmetrical) but I'm leaving this issue open to ask if it should fail to resolve in both cases.

I agree that the asymmetry seems clearly wrong. I'm trying to think about what I'd expect the behavior to be, though. Thinking out loud, I imagine three possible interpretations:

  1. r32 and r64 are different types, so trying to do == on two such records should result in a compile-time error by default. If the user wants to do comparisons across different instantiations of R, they should write the == operator themselves.

  2. The default comparison should compare all fields including the type field, in which case it should resolve, but return false for this case.

  3. The default comparison should compare all execution-time fields of the record, in which case it should resolve, and return true for this case since we support == between int(32) and int(64) (via coercion of the 32-bit value).

If I were to rank these choices, I think I'd rank them as 1, 3, 2. But I think the expectation between 2 and 3 could be confusing enough that I think 1 is probably the better choice.

If we have two record types with identical fields - can we compare their instances?

record R1 { var x: int; }
record R2 { var x: int; }
var r1: R1, r2: R2;
writeln(r1==r2);

If yes, then any instantiations of G should be comparable.

I thought that the defaults for record comparison and assignment look only at record fields, not the record types per se. Under that philosophy, it should be "yes" above.

Should we include "compile-time" fields in the comparison?

I think we should, because otherwise two records that differ only in a param field would be the same, ex:

record R { param x; }
var r1: R(1); var r2: R(2);
writeln(r1 == r2);

I suggest that comparison of records differing in compile-time fields generates a compile-time "false".

Another twist is comparing two records with like-named field that is param in one and type in the other. Since == over this combination does not resolve, == over such two records should not resolve either.

If we have two record types with identical fields - can we compare their instances?

I assume from your question that you didn't try this? Running your example shows that no, we can't. And I think this is appropriate鈥攊f I have an "age" record and an "SAT score" record, it doesn't make sense to support comparisons between them automatically even if they happen to store the same fields.

Your response increases my confidence that not supporting comparisons by default between different generic instantiations would be the least surprising given that you preferred my option 2 over 3 where I'd gone the opposite way. The author of a record can always provide the comparison if they want to support either 2 or 3.

Disallowing comparison between records of different types makes sense to me. In which case we should go with Option 1.

By the same reasoning it make sense to disallow default assignment between records of different types or different instantiations of the same generic type.

For both assignment and comparison, it would be nice to generate a specific error message instead of a generic "cannot resolve".

P.S.

I assume from your question that you didn't try this?

I was/am thinking "from scratch" rather than from whatever is currently implemented.

Speaking about assignments, what is currently implemented is:

  • we can't assign from age to SAT
  • we can't assign from r64 to r32
  • we CAN assign r64 = r32 in Michael's original example

I suggest that the last one should be fixed to be disallowed.

I am in favor of the more restrictive option 1 as discussed above.

Open question - Should these rules apply to classes as well?

Nevermind, we do pointer comparisons on classes.

  • [ ] try disallowing assignments such as r64/r32
  • [ ] try disallowing comparisons such as r64/r32. Check other comparison operatiors.

I'm seeing 64 errors from not allowing the = across different types like this:

> $CHPL_HOME/modules/dists/CyclicDist.chpl:776: In function 'dsiAccess':
> $CHPL_HOME/modules/dists/CyclicDist.chpl:801: error: type mismatch in assignment from _remoteAccessData(int(64),1,int(64),true,false) to _remoteAccessData(int(64),1,int(64),false,false)

e.g. optimizations/sungeun/RADOpt/access1d.chpl

TODO: try passing locDom.myBlock.stridable to LocRADCache type constructor on line 951.

Was this page helpful?
0 / 5 - 0 ratings