[WIP] Refactor node weave #2557
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These were missing a lot of edge cases.
|
The |
|
@mgreter I have a branch (#2775) where I've rewritten selector unification to match Dart Sass. This fixes a ton of bugs but unfortunately breaks Perhaps you can make a PR that just implements subweave for AST nodes, then I can try updating |
This was referenced Nov 29, 2018
|
Update: My unification rewrite has been merged, it's all good. |
glebm
reviewed
Dec 10, 2018
| } | ||
|
|
||
| template <class T> | ||
| bool SetContains(const T& lhs, const T& rhs) { |
There was a problem hiding this comment.
How about using hash maps/sets throughout (std::unordered_map)? They are faster and easier to get right
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a early WIP branch trying to fix the very old ast<->node implementation nobody understands. Subweave is already pretty much working ok, but still some more work to do. One can see that the initial architecture for complex_selectors is not very optimal, since we need to copy/clone the whole linked list chain (tails) once we want to change connections. For that I have introduced
Selector_Groupsfor now. Putting this here since it was laying around for quite some time in my local repo.