Branch: refs/heads/main
Revision: 29fa80d7f4ecfabcd13a330b2d0a734de79fe351
Author: bradcray
Link: Fix a postinit case for unions; customize union errors for management types by bradcray · Pull Request #29281 · chapel-lang/chapel · GitHub
Log Message:
Fix a postinit case for unions; customize union errors for management types (#29281)
While prepping my demo for this morning's Chapel meeting, I ran across a
case where we weren't calling postinit() as we should've been. In fact,
one of the tests I added in #29253 had this bug, but it slipped past
both me and my reviewer. It seems that when declaring a union field like
var myU: u; we did not call postinit(). The fix for this was simple,
and reflected another isRecord() test that should've been expanded to
include isUnion().
This PR fixes that issue and also extends a few other 'isRecord()' tests
in functionResolution.cpp to include unions as well. The most visible of
these is that we now generate a customized error for (illegal) 'new
owned u()' and friends, and I've added a test to lock that in. Here, the
case was not as bad for non-unions, as we had a catch-all for "other
non-record, non-class types."
Of the changes here, the one to line 13276/9 is pretty speculative. I
spent a few minutes trying to write a test showing that it improved
things, but wasn't able to do so quickly. My sense is that if this case
should include unions (as I'm hypothesizing), it's easier to fix it now
than to hunt it down later; and that if it shouldn't, it'll either bite
us in an obvious way or be innocuous.
[reviewed by @benharsh ]
Diff:
M compiler/resolution/functionResolution.cpp
A test/types/unions/union-new-managed.chpl
A test/types/unions/union-new-managed.good
M test/types/unions/union-postinit.good
https://github.com/chapel-lang/chapel/pull/29281.diff