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

Lift the "field must have the same type in all constructors" restriction [was: Field name conflict bug in Enum]. #994

Open
qishen opened this issue Jun 9, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@qishen
Copy link

qishen commented Jun 9, 2021

I'm trying to define a new type that could be either string or integer. My first try is to write typedef NID = string | u32 but apparently not supported by ddlog so I came up with this one typedef NID = StrId {nid: string} | NumId {nid: u32} . The error message I got is error: graph.dl:10.15-11.1: Field nid is declared with different types saying the StrId and NumId cannot have the same field but it does not make sense since they are different constructors.

It could be a bug in the compiler and I also wonder what's the best way to write a union type that represents the union of multiple primitive types?

The other similar question is how to reuse the types defined in the relations if I want a union type like typedef Item = Node | Edge where input relation Node(id: u32) input relation Edge(src: Node, dst: Node)

@ryzhyk
Copy link
Contributor

ryzhyk commented Jun 9, 2021

Ddlog requires that fields with different types in different constructors have different names.

@ryzhyk
Copy link
Contributor

ryzhyk commented Jun 9, 2021

For the other question,

typedef Item = ItemNode{node: Node} | ItemEdge{edge: Edge}

Apologies about brevity and poor formatting. Typing on the phone.

@ryzhyk
Copy link
Contributor

ryzhyk commented Jun 9, 2021

Ddlog requires that fields with different types in different constructors have different names.

By the way, this restriction was borrowed from Haskell and may not actually be necessary in ddlog. We may get rid of it in the future.

@qishen
Copy link
Author

qishen commented Jun 9, 2021

Thanks for answering me on your phone! I still don't have time (mainly because of procrastination) to look at Compile.hs and make changes for the previous PR yet but will try to figure it out and maybe help to remove this field name restriction too.

@ryzhyk
Copy link
Contributor

ryzhyk commented Jun 9, 2021

I was hoping we'd convinced you to use ddlog instead of its internals :). There's unfortunately way too much on my to-do list to help with that feature in the near future.

As for the field name restriction, it's not just a matter of removing the check. We should also make sure it doesn't break type inference or any other part of the compiler, add tests for it, etc.

@mihaibudiu mihaibudiu added the enhancement New feature or request label Jun 9, 2021
@qishen
Copy link
Author

qishen commented Jun 9, 2021

@ryzhyk You are very close to convincing me to use ddlog directly. I tried my own implementation built upon internals but does not work well. I'm still exploring the potential of ddlog to suit my needs in FORMULA and figure out if they have the exact same semantics despite using the same or similar syntax. One of the great things FORMULA has is nested aggregation (aggregation when certain constraints are satisfied but those constraints also have aggregation inside them) even though it's not used very often. The aggregation in ddlog seems to be highly customizable and I'll do more experiments on it.

If you can't beat 'em, join 'em

Maybe at the end I can just add a feature to ddlog if the feature I need does not exist or a wrapper on top of ddlog. We will see.

Do you guys have a slack channel for ddlog users?

@ryzhyk
Copy link
Contributor

ryzhyk commented Jun 9, 2021

There's a gitter link in the readme. Or feel free to use the discussions section on GitHub.

@ryzhyk ryzhyk changed the title Field name conflict bug in Enum Lift the "field must have the same type in all constructors" restriction [was: Field name conflict bug in Enum]. Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants