perf: precompute intersections (2x speedup)

[Eh2406]

This is a cleaned up version of @mpizenberg's work in #33 (comment)

[mpizenberg]

Looks good to me

[mpizenberg]

Woops sorry for closing this one. I was cleaning my unused branches and didn't realized the last commit was still mine here. I thought @Eh2406 would have done a new commit.

[aleksator]

I'll review in the morning.

[aleksator]

This must be working as intended since tests pass, but I would like to ask for my own understanding.
std::mem::take replaces self.history with a default value, i.e. empty Vec in this case. Before the change the history was retained.

Why doing this is correct in this case?

[Eh2406]

Because we are building a new self and its history and assigning it to *self.

old code:

let copy = self.history.to_owned();
let new_self = Self::from_assignments(copy);
*self = new_self;

Note how no one uses the self.history after the copy. It is just replaced as part of the last line.

[aleksator]

Didn't see that, thank you for the explanation!

[mpizenberg]

@aleksator why do we say term_intersection if we say "intersection of terms"? I'm really bad at grammar so I don't even know how to search for an explanation for that.

[aleksator]

@aleksator why do we say term_intersection if we say "intersection of terms"? I'm really bad at grammar so I don't even know how to search for an explanation for that.

The rule of thumb in my understanding is yes. Let me try to google where I got that from... The search term I'm going to use is plural nouns as adjectives (I'm rather literal 😆).

First link from Duckduckgo is ambiguous, just saying that it can be either, but usually singular. It says plural is for nouns that usually refer to people or are mostly used in plural form; term is neither of those.

Second link defines a proper name for this which could be used for later searches: noun adjuncts, or attributive nouns.
It also gives a bunch of rules, but also ends with "whatever native speakers would use".

It is rather unhelpful 🙄, but that's what I generally use as a basis for my changes as well. If it sounds way off when I read it, it probably should be changed. If both ways sound clear or both sound a bit weird to my ear, then I leave it as is.

Disclaimer: I'm not a native speaker myself, although everything that could be done using English I do in English for the past decade or so. That only leaves verbal communication with surrounding people who don't speak English that has to be done in my native language, so I often find myself having to just drop in English words in my speech rather than pause trying to remember what the translation is. 😅

[aleksator]

An example of a function I didn't rename and why:
other_terms_intersection

Here if we rename it to other_term_intersection (without s) it would seem like we are trying to say another_(term_intersection) instead of intersection_of_(other_terms).

While intersection_of_other_terms reads smoother than other_terms_intersection, I thought that maybe it's better to keep the naming a) shorter and b) more consistent (we don't use of in other similar functions). Hence no changes this one.

[Eh2406]

By the way, I am a native speaker, but my spelling and grammar are atrocious. So I deeply appreciate your correction of any mistakes you find.

[aleksator]

By the way, I am a native speaker, but my spelling and grammar are atrocious. So I deeply appreciate your correction of any mistakes you find.

Thank you! I hope I'm not too pedantic with this stuff 😄