Branch: refs/heads/main
Revision: a0eb2cafe09126ef083c1fa5dab929f1bfc439d6
Author: bradcray
Link: Define record comparisons as module code in preview edition by bradcray · Pull Request #28731 · chapel-lang/chapel · GitHub
Log Message:
Define record comparisons as module code in preview edition (#28731)
[reviewed by @DanilaFe]
This is a change to the preview edition, motivated by #28699, which
moves record comparison operators into module code rather than defining
them on a type-by-type basis in the compiler. This idea is captured in
#28708 where the motivations include (a) simplicity of implementation
and reading/modifying the implementation, (b) not relying on compiler
heuristics to determine whether the user defined these operators
themselves or not, essentially changing an operator definition detection
challenge into "just another case of resolution." Though this change is
only enabled in the preview edition, during development, I tested it
against the full suite and found it to pass (modulo the changes to error
messages, which I consider an improvement).
Specifically, ChapelBase.chpl now contains operator overloads of the
form:
operator ==(a: record, b: record) { … }
operator !=(a: record, b: record) { … }
operator <(a: record, b: record) { … }
etc.
each of these are marked pragma "last resort" to make them act as a
catch-all if no better matches are found. It is likely that this pragma
could be removed in the future if #28751 was implemented.
These overloads also generate an error if a.type != b.type, which
results in an arguably clearer error message than the current generic
"failure to resolve" errors we get when comparing two records which
don't support that comparison.
One interesting question that Daniel asked along the way is what the
behavior of these operators should be with type fields that have runtime
type information (such as an array type field)? There didn't seem to be
an easy way to do something smart and reasonable here, so I forked this
off into an issue and some new tests and futures instead:
How should compiler-generated record comparison/equality operators interact with runtime type fields · Issue #28755 · chapel-lang/chapel · GitHub At present, this PR
just skips over type fields, as do the current compiler-generated
operators.
This PR also contains an update to the test system to specialize the
error message for a correctness test that fails to compile, does not
have an explicit .noexec file, yet for which the expected .good file
cannot be found. Previously, it would just state that compilation
failed—which was what I was expecting and wanting to have happen—without
showing the diff or the fact that it couldn't find the .good file to
diff against. In my case, this was because I'd mis-typed the name of the
.good file in my .compopts file, and the test system didn't mention
the lack of the .good file because the same error was shared by
performance testing, which has no .good files in general. Here, I
split the two cases apart in order to mention the lack of the .good
file explicitly.
In a bit more detail, the changes are:
code changes:
- modules/internal/ChapelBase.chpl: added the overloads to the end of
the file - compiler/passes/buildDefaultFunctions.cpp: guarded the
compiler-generated operators by a preview edition check
docs changes:
- doc/rst/language/spec/records.rst, doc/rst/technotes/editions.rst:
update docs to define new behavior in preview edition
new tests:
- test/types/records/comparisons/compareRecords2.chpl: a new test to
make sure all record comparisons behave as expected; - test/types/records/comparisons/emptyRecordComp.chpl: a new test making
sure empty record comparisons work as expected - test/types/records/comparisons/recordTypeParamComp.chpl: a new test
checking that type/param fields don't affect comparisons, as expected
variants of existing tests: