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

Warnings-- #395

Merged
merged 1 commit into from
Jun 10, 2014
Merged

Warnings-- #395

merged 1 commit into from
Jun 10, 2014

Conversation

kornelski
Copy link
Contributor

Fixes some cases where clang complains about loss of integer precision.

Plus fixes call to abs that was considered ambiguous.

akhleung pushed a commit that referenced this pull request Jun 10, 2014
@akhleung akhleung merged commit c342355 into sass:master Jun 10, 2014
@kornelski kornelski deleted the analyzer branch June 10, 2014 17:46
@andrew
Copy link

andrew commented Jun 10, 2014

I may be missing something here, but I'm still getting the same build error from abs that was considered ambiguous on lucid-x64: https://gist.github.com/andrew/30eaff7e65e5e4fd0f65

../libsass/utf8_string.cpp: In function 'size_t Sass::UTF_8::normalize_index(int, size_t)':
../libsass/utf8_string.cpp:111: error: call of overloaded 'abs(int&)' is ambiguous
/usr/include/c++/4.4/cmath:94: note: candidates are: double std::abs(double)
/usr/include/c++/4.4/cmath:98: note:                 float std::abs(float)
/usr/include/c++/4.4/cmath:102: note:                 long double std::abs(long double)
make: *** [Release/obj.target/binding/libsass/utf8_string.o] Error 1

Upgrading glibc would fix it, but then the binary won't work on heroku, which uses glibc-2.11

Any suggestions?

@kornelski
Copy link
Contributor Author

Hmm, do you think cast to double would be better?

@andrew
Copy link

andrew commented Jun 10, 2014

@pornel yeah I just tried that out and it builds cleanly for me

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.

3 participants