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

Renaming function whose output is an E-class to constructor #422

Open
yihozhang opened this issue Sep 6, 2024 · 3 comments
Open

Renaming function whose output is an E-class to constructor #422

yihozhang opened this issue Sep 6, 2024 · 3 comments
Assignees

Comments

@yihozhang
Copy link
Collaborator

yihozhang commented Sep 6, 2024

To emphasize the differences between functions whose output is an E-class ("constructor" function), which has unique semantics*, and other functions, we decided to change the syntax for declaring functions whose output is an E-class to use keyword constructor. So what used to be

(datatype E)
(function Add (E E) E)

should now be written as

(datatype E)
(constructor Add (E E) E)

Some non-constructors:

(datatype E)
(function best (E E) E :merge new) ;; because there is a merge expression
(function add1 (i64) i64) ;; because output is a primitive (and merge expression is an implicit `panic`)
(function upper-bound (E) i64 :merge (max old new)) ;; because it has a merge function

We should try to support this new constructor without breaking existing programs. So we should still support existing forms like (function Add (E E) E) but issue a warning to prompt the user.

*: constructor functions can be looked up in actions (cf. #420 for non-constructors) where it will be created on the fly (cf. #421 for non-constructors)

@oflatt
Copy link
Member

oflatt commented Sep 10, 2024

I was under the impression that we were making a breaking change- existing programs must re-name to constructor.
(function Add (E E) E) would not be supported. I believe our rational was that egglog is still quite young, so we want to make these changes while we can.

@yihozhang
Copy link
Collaborator Author

Sure, I think there are three paths we can take

  1. Simplify disallowing (function Add (E E) E) in our parser.
  2. Emitting a nicer error for (function Add (E E) E), rejects the program, and suggests the user use constructor.
  3. Emitting a warning for (function Add (E E) E), still accepts the program, and suggests the user use constructor.

Although my original issue suggested (3), I'm fine with either one. Doing (1) is all good, although (2) and (3) are bonus points that don't take a lot of effort but can improve user experience (and can potentially avoid us some Zulip questions about unexpected errors).

@oflatt
Copy link
Member

oflatt commented Sep 11, 2024

2 would be awesome, it's what I was imagining! A little blurb explaining the difference between the two in an error message would go a long way. Same for the documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants