Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect implementation of pattern matching for exceptions #223

Closed
1 of 5 tasks
JohnReppy opened this issue Jul 16, 2022 · 0 comments
Closed
1 of 5 tasks

Incorrect implementation of pattern matching for exceptions #223

JohnReppy opened this issue Jul 16, 2022 · 0 comments
Assignees
Labels
bug Something isn't working compiler problem with compiler fixed-in-2022.1 issue is fixed in the 2022.1 release gforge bug (or feature request) ported from smlnj-gforge repository

Comments

@JohnReppy
Copy link
Contributor

Version

110.99.1

Operating System

  • All
  • Linux
  • macOS
  • Windows
  • Other Unix

OS Version

No response

Processor

Any

Component

Core system

Severity

Minor

Description of the problem

The example comes from the coresml regression tests (test d001a-ac.sml).

With FLINT printing on, we see the source of the problem is that the order of cases has changed (B appears before A): For exception constructors, which can be aliases, this change results in incorrect behavior.

[After Translate ...]

FN(v85:
STR([TYC(TCAP(PRIM(ETG), [{}])),TYC(TCAP(PRIM(ETG), [{}])),
TYC(TCAP(PRIM(ETG), [{}]))]),
v73 = v85[0]
v72 = v85[1]
v74 = v85[2]
v70 =
v71 =
CON((B,EXNLVAR(72),TYC(AR[rr]([{}], [PRIM(EXN)]))), [],
RCD())
v75 = FN(v76: TYC({}),(I63)1)
v77 = FN(v78: TYC({}),(I63)2)
v79 = FN(v80: TYC({}),(I63)3)
v81 =
FN(v82:
TYC({}),
RAISE(TYC(PRIM(I63)),
APP(PRM(markexn,
TYC(AR[rr]([PRIM(EXN),PRIM(STR)],
[PRIM(EXN)])), []),
RCD(CON((Match,EXNLVAR(74),TYC(AR[rr]([{}],
[PRIM(EXN)]))), [], RCD()),
"stdIn:4.36"))))
SWIv71
of B.v83 => APP(v77, RCD())
A.v84 => APP(v75, RCD())
_ => APP(v79, RCD())
SRCD(v70))

Transcript

<jhr@Froh> sml
Standard ML of New Jersey (64-bit) v110.99.1 [built: Mon Apr 12 18:45:14 2021]
- exception A;
exception A
- exception B = A;
exception B = A
- (case B of A => 1 | B => 2 | _ => 3);
val it = 2 : int

Expected Behavior

No response

Steps to Reproduce

exception A;
exception B = A;
(case B of A => 1 | B => 2 | _ => 3);

Additional Information

No response

Email address

No response

Comments from smlnj-gforge

Original smlnj-gforge bug number 290

Submitted on 2021-57-10 at 20:5700

Keywords: pattern matching, exceptions

comment by @dmacqueen on 2021-06-10 03:0600 +000 UTC

A spurious "rev" operation in the SWITCHexp case of Translate.mkExp was removed, which restored the proper order of rules in the example (the A => rule ~ now comes first. However, this was not enough to fix the bug. Deeper analysis of the issues around identity exception declarations will be required.

comment by @dmacqueen on 2022-23-17 19:2300 +000 UTC

The mkExp rule transposition problem still applies the the legacy branch, but with the new match compiler, the bug persisted. The fix is to suppress the SWITCH contraction (FLINT/opt/lcontract.sml: swiInfo; FLINT/opt/fcontract.sml: fcSwitch) when the subject is an exception constructor, because these functions use equality of conreps to identify the exception constructor, which is not correct when one of the exceptions is defined by an exception identity declaration as in the bug test case. The bug remains open in the legacy branch.

@JohnReppy JohnReppy added bug Something isn't working compiler problem with compiler gforge bug (or feature request) ported from smlnj-gforge repository labels Jul 16, 2022
@JohnReppy JohnReppy added the fixed-in-2022.1 issue is fixed in the 2022.1 release label Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler problem with compiler fixed-in-2022.1 issue is fixed in the 2022.1 release gforge bug (or feature request) ported from smlnj-gforge repository
Projects
None yet
Development

No branches or pull requests

2 participants