Strum / Strums_Mental_VCV_Modules

Strum's Mental Modules for VCV Rack
BSD 3-Clause "New" or "Revised" License
63 stars 15 forks source link

Replace occurances of C style abs() with std::abs() #25

Closed thetasine closed 6 years ago

thetasine commented 6 years ago

Compiling errors out on MentalMuxes.cpp, MentalFold.cpp, and MentalClip.cpp due to use of abs() instead of std::abs(). Replacing the function allows the module to be built.

Strum commented 6 years ago

what platform?

thetasine commented 6 years ago

@Strum My fault, I guess I forgot that piece of info. Windows MinGW-w64 (msys2-x86_64-20161025). Using VCV Rack 0.5 release.

Strum commented 6 years ago

that's funny compiles fine for me

thetasine commented 6 years ago

I'll double check if there are any differences in my files compared to the repository. I'll have to do this later, at work atm.

phdsg commented 6 years ago

on my linux machine i have to change the abs for a succesful build. my msys2 doesn't care either way.

thetasine commented 6 years ago

Back home. I cloned the v0.5 VCV Rack branch, and your plugin to a new development environment, and ran through the standard compile steps to be on the safe side. I still get an error, which ends stops on MentalMuxes.

Here is an errorlog of MentalMuxes to start with. Replacing abs() with std::abs() allows it to compile successfully. Make will stop again at MentalFold until patched.

I'm using Windows 10 v1703, build 15063.726 fwiw.

If you wish, I can push fixes if you feel it isn't going to hurt.

Strum commented 6 years ago

easy enough to fix but i'm curious as to why you are getting errors when I am not in the same environment.

thetasine commented 6 years ago

I'm not sure either tbh.

Within Rack, include/math.cpp changed the include from "math.h" to "cmath" (commit). This should allow abs to work as expected.

And, just to be safe, I searched the entire Rack source for inclusion cstdlib and found nothing. This could sometimes reverse the behavior back to C style abs if it was included before your plugin.

If I could ask you to verify that you are using MSYS2 release msys2-x86_64-20161025 and not anything else, that could narrow it down. If you are using a different release, then the it could be possible the GCC compiler implementation is different, and is introducing this bug somewhere. Although this is really unlikely as low level headers like this shouldn't really change that often.

Fyi, MSYS2 does have a release on SF that is older than what is on their webpage, from September.