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

Improper merging of unequal keys in map-merge #1283

Closed
davidkpiano opened this issue Jun 18, 2015 · 5 comments
Closed

Improper merging of unequal keys in map-merge #1283

davidkpiano opened this issue Jun 18, 2015 · 5 comments

Comments

@davidkpiano
Copy link

TEST:

$map: ();

$map: map-merge($map, (1 2: 3));
$map: map-merge($map, (2 1: 3));

.test {
  test: inspect($map);
}

EXPECTED: (Ruby Sass)

.test {
  test: (1 2: 3, 2 1: 3);
}

ACTUAL: (LibSass 3.2.5)

.test {
  test: (1 2: 3);
}

I'll write specs for all these issues soon, I promise!

@mgreter mgreter added this to the 3.3 milestone Jun 23, 2015
@mgreter mgreter self-assigned this Jun 23, 2015
@mgreter
Copy link
Contributor

mgreter commented Jun 23, 2015

Thxs for the report @davidkpiano! I can confirm this issue and also have a fix ready!

In ast.hpp class List virtual size_t hash():

- hash_ ^= (elements()[i])->hash();
+ hash_ ^= (elements()[i])->hash() + (i + 1);

mgreter added a commit to mgreter/libsass that referenced this issue Jun 23, 2015
mgreter added a commit to mgreter/libsass that referenced this issue Jun 23, 2015
@mgreter
Copy link
Contributor

mgreter commented Jun 25, 2015

First solution approach was too naive, should use this instead:

  template <typename T>
  void hash_combine (std::size_t& seed, const T& val)
  {
    seed ^= std::hash<T>()(val) + 0x9e3779b9
             + (seed<<6) + (seed>>2);
  }
...
      for (size_t i = 0, L = length(); i < L; ++i)
        hash_combine(hash_, (elements()[i])->hash());

xzyfer added a commit to xzyfer/sass-spec that referenced this issue Jun 29, 2015
@xzyfer
Copy link
Contributor

xzyfer commented Jun 29, 2015

Specs added sass/sass-spec#414

xzyfer added a commit to xzyfer/sass-spec that referenced this issue Jul 8, 2015
@okdana
Copy link

okdana commented Jul 8, 2015

Thank you for fixing this!

@xzyfer
Copy link
Contributor

xzyfer commented Jul 8, 2015

This is fixed and will be in 3.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants