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

allow per-language discriminants #270

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

bjchambers
Copy link
Contributor

For languages like SymbolLang that don't have an outer-most enum this allows defining a discriminant and reducing the set of nodes considered for matching. This allows each Language to determine the type to use for the discriminant and how to compute it.

For instance:

  1. This may be a std::mem::Discriminant<Self> if the type is an enum
  2. This may be a u64 if it is easy to hash the "operation"
  3. This may be Symbol for something like SymbolLang (or a discriminant of the operation, in the case of something like SymbolLang but using an enum for the symbol)
  4. This may be something a bit richer for cases like ENodeOrVar.

This also allows languages to trade-off how rich the discriminant is with how efficient it is to compute / compare.

@mwillsey
Copy link
Member

This looks great! Could you please document the associated type as well as in the define_language macro that it's using the enum discriminant.

@bjchambers
Copy link
Contributor Author

This looks great! Could you please document the associated type as well as in the define_language macro that it's using the enum discriminant.

Absolutely. Wanted to get it out and make sure it seemed reasonable before polishing it too much. Will do so now!

while start > 0 {
if std::mem::discriminant(&eclass.nodes[start - 1]) == discrim {
if eclass.nodes[start - 1].discriminant() == discrim {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure about this check here. Why is it expected that nodes with equal discriminant will be adjacent? It doesn't seem like the nodes are sorted by discriminant (the Discriminant doesn't implement Ord).

@bjchambers
Copy link
Contributor Author

Comments added.

@mwillsey mwillsey merged commit a84c3c4 into egraphs-good:main Nov 28, 2023
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants