Defer creation of hash maps into eval stage #1736
Merged
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 finally fixes #1169, which was caused by a severe bug with how we handle maps!
Additional Spec-Tests: sass/sass-spec#623
The current behavior seems rather random and I didn't care to check when exactly it wouldn't work as expected. The bug is rather obvious, as can be seen with the following example:
This will currently result in a duplicate key error. The parser actually already creates a map for the given sass code above, which will never work correctly, since we only know the actual keys after the evaluation phase (which is not available to the parser). It feels rather natural to use an even-sized list to carry it over to the evaluation phase, and create the actual map there.
A lot of languages allow the creation of maps from even-sized lists, so it felt natural to extend the existing List class to support this. But we only use this internally (from an external point, we only have space and comma separated lists). Unsure if sass can convert hash maps to lists and vice versa?
The error reporting is once again rather funky. It will always report the hex representation for duplicate color keys. This is the first time I see this happen and our codebase didn't support that in anyway without adding a new function just to get that representation (it's not
to_string
norinspect
).ToDo:
to_string
for missing expression typesto_string
functions in CRTP visitors