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

include cstdlib for std::abs() for Emscripten #388

Merged
merged 1 commit into from
Jun 7, 2014

Conversation

rodneyrehm
Copy link
Contributor

While gcc compiled fine, emscripten did not. The following error was thrown:

/usr/local/bin/emscripten/emscripten/1.8.2/em++ -Wall -O2 -fPIC -c -o utf8_string.o utf8_string.cpp
utf8_string.cpp:111:16: error: call to 'abs' is ambiguous
      else if (std::abs(index) <= signed_len) {
               ^~~~~~~~

Either including cstdlib or changing std::abs(index) to std::abs((float)index) fixed the issue with Emscripten without breaking in gcc.

While gcc compiled fine, emscripten did not. The following error was thrown:

```
/usr/local/bin/emscripten/emscripten/1.8.2/em++ -Wall -O2 -fPIC -c -o utf8_string.o utf8_string.cpp
utf8_string.cpp:111:16: error: call to 'abs' is ambiguous
      else if (std::abs(index) <= signed_len) {
               ^~~~~~~~
```

Either including `cstdlib` or changing `std::abs(index)` to `std::abs((float)index)` fixed the issue with Emscripten without breaking in gcc.
@LaurentGoderre
Copy link
Contributor

👍 was about to suggest the same

@LaurentGoderre
Copy link
Contributor

Hmmm, I was wondering why the CI build failed but then again all of the previous build failed as well. Not very helpfull

@HamptonMakes
Copy link
Member

It fails because we haven't passed sass/sass-spec yet. Damnit.

On Fri, Jun 6, 2014 at 4:06 PM, Laurent Goderre notifications@github.com
wrote:

Hmmm, I was wondering why the CI build failed but then again all of the
previous build failed as well. Not very helpfull

Reply to this email directly or view it on GitHub
#388 (comment).

@akhleung
Copy link

akhleung commented Jun 6, 2014

Sorry, there are a few @extend-related regressions, which is why the tests are failing. I'll move them to the todo bucket this weekend.

@LaurentGoderre
Copy link
Contributor

Hmm, it seems to me like some of the build failure are compiling errors

@akhleung
Copy link

akhleung commented Jun 6, 2014

Ah, I see the problem ... the build that uses the default makefile compiles, but fails on the aforementioned test cases. The one that uses autotools fails to compile because I haven't updated the makefile template to include some new files that were added in a recent commit.

@utkarshkukreti
Copy link
Contributor

I was getting the same error as OP when compiling using clang, and this patch fixed it. 👍!

akhleung pushed a commit that referenced this pull request Jun 7, 2014
include cstdlib for std::abs() for Emscripten
@akhleung akhleung merged commit 2099d01 into sass:master Jun 7, 2014
@LaurentGoderre
Copy link
Contributor

👍

@LaurentGoderre
Copy link
Contributor

It appears this fix is incomplete and it still fails on Heroku. This fix probably need to be applied to this file as well

https://github.com/sass/libsass/blob/483124834a75b1ad269e2555272ab96ad3296706/functions.cpp

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.

5 participants