Fix a compilation issue on mac OS #403
Conversation
|
Hi Alex, |
Hi Guy, Now that I moved to FloatUtils, travis build fails... However, it builds perfectly well both on my OS and on the linux distribution in my office. Any ideas? We can, of course, always move the include back to SignConstraint... |
|
Hi Alex,
I think we should debug this.
Are you using the exact same flags as Travis? Did you try with both the gcc
and clang compilers?
Did you Google the errors?
From a quick look, you're including both <cmath> and <math.h>; I think the
former is the "c++ variant" of the latter, and I'm not sure whether
including them both can cause strange behaviors.
…On Wed, 11 Nov 2020 at 18:14, alexopenu ***@***.***> wrote:
Hi Alex,
Thanks for the PR. Yes, we can just put it in FloatUtils.
Hi Guy,
Now that I moved to FloatUtils, travis build fails... However, it builds
perfectly well both on my OS and on the linux distribution in my office.
Any ideas? We can, of course, always move the include back to
SignConstraint...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#403 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADSTPXJIRN3GNIFH2RQR2N3SPKZ5HANCNFSM4TO33XWA>
.
|
|
|
Sure, I just wasn't sure whether you preferred making a bigger change, given that it had worked fine with math.h except for one problem that could be solved locally; but I agree that it is better to solve the issue globally. I think it is fine now, all tests pass, so if you are happy, I will merge it. (sorry, accidentally closed and reopened the pr earlier)
|
|
Looks good, you can go ahead and merge.
…On Thu, 12 Nov 2020 at 09:25, alexopenu ***@***.***> wrote:
Sure, I just wasn't sure whether you preferred making a bigger change,
given that it had worked fine with math.h except for one problem that could
be solved locally; but I agree that it is better to solve the issue
globally. I think it is fine now, all tests pass, so if you are happy, I
will merge it.
(sorry, accidentally closed and reopened the pr earlier)
Hi Alex, I think we should debug this. Are you using the exact same flags
as Travis? Did you try with both the gcc and clang compilers? Did you
Google the errors? From a quick look, you're including both and <math.h>; I
think the former is the "c++ variant" of the latter, and I'm not sure
whether including them both can cause strange behaviors.
… <#m_-5088526561561348457_>
On Wed, 11 Nov 2020 at 18:14, alexopenu *@*.***> wrote: Hi Alex, Thanks
for the PR. Yes, we can just put it in FloatUtils. Hi Guy, Now that I moved
to FloatUtils, travis build fails... However, it builds perfectly well both
on my OS and on the linux distribution in my office. Any ideas? We can, of
course, always move the include back to SignConstraint... — You are
receiving this because you were mentioned. Reply to this email directly,
view it on GitHub <#403 (comment)
<#403 (comment)>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ADSTPXJIRN3GNIFH2RQR2N3SPKZ5HANCNFSM4TO33XWA
.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#403 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADSTPXO26UGEWIQA3OHK2T3SPOEYHANCNFSM4TO33XWA>
.
|
* Include cstdlib. * Include cmath instead. * Remove cstdlib. * Include cmath globally in FloatUtils instead of locally. * Trigger * Attempt to fix the cmath vs math.h incompatibility issue.
This pr addresses a similar compilation error on a mac to the one that was fixed in #294.
Like before, including cmath solves the ambiguity issue of the call to abs(). @guykatzz, do you think it would make more sense to include it in a different header, more globally (e.g., FloatUtils)?