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

Prbs compile on Mac #5

Open
Motopomp opened this issue Jan 1, 2018 · 12 comments
Open

Prbs compile on Mac #5

Motopomp opened this issue Jan 1, 2018 · 12 comments

Comments

@Motopomp
Copy link

Motopomp commented Jan 1, 2018

no problem to compile it on Win and Linux , but lots of errors on Mac…
See screenshot

capture d ecran 2018-01-01 a 11 28 08

@Motopomp Motopomp changed the title Prbs copile on Mac Prbs compile on Mac Jan 1, 2018
@Aepelzen
Copy link
Owner

Aepelzen commented Jan 1, 2018

Thanks for the info. I've had this problem before with somebody trying to build this with XCode.
mode is a template Parameter, and apparently XCode does strange things with those. May i ask what compiler you used?

However, I don't think i can do much about those errors. For one, i do not have a mac to verify any changes. Second, the error happens while compiling the parasites firmware which is not part of my repository (only included as a subtree). Any changes to that would have to happen in the parasites repository.

@Motopomp
Copy link
Author

Motopomp commented Jan 1, 2018

Re,
For the compilation method, I do not use Xcode itself. I use the Andrw method described on VCV page (CMake and Homebrew), so I think it's Gcc.
I specify you in case, I am not a developer, so apart from following indications and return errors found I could not correct the errors. On the other hand do not hesitate to ask any info whatsoever, I can test any fix you could bring
Ps: happy new year
Ps2: sorry for my english, ggle tries to help me ......;)

@JerrySievert
Copy link

it's clang++, not gcc/g++.

specifically (and it's right, btw):

inline GeneratorMode mode() const { return mode_; }

you can't do something like this:

    uint16_t x = mode == GENERATOR_MODE_AR ?

I got it to compile by changing mode to mode() in the upstream module, but I don't have the hardware either, so can't verify if it's a fix or not, just that it compiles (though, looking at the definition of mode(), it likely does what it's supposed to.

not sure why g++ ignores the standard for this.

@lnikj
Copy link

lnikj commented Jan 8, 2019

it's clang++, not gcc/g++.

specifically (and it's right, btw):

inline GeneratorMode mode() const { return mode_; }

you can't do something like this:

    uint16_t x = mode == GENERATOR_MODE_AR ?

I got it to compile by changing mode to mode() in the upstream module, but I don't have the hardware either, so can't verify if it's a fix or not, just that it compiles (though, looking at the definition of mode(), it likely does what it's supposed to.

not sure why g++ ignores the standard for this.

Could you tell me where that edit is Jerry? I have the hardware so can build and test.

These are the errors I'm currently getting (in Mojave):

parasites/tides/generator.cc:400:21: warning: comparison of integers of
different signs: 'uint32_t' (aka 'unsigned int') and 'int'
[-Wsign-compare]
if (end_of_attack >= abs(phase_increment)) {
~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~
parasites/tides/generator.cc:403:21: warning: comparison of integers of
different signs: 'uint32_t' (aka 'unsigned int') and 'int'
[-Wsign-compare]
if (end_of_attack < abs(phase_increment)) {
~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~
parasites/tides/generator.cc:557:20: warning: comparison of integers of
different signs: 'uint32_t' (aka 'unsigned int') and 'int'
[-Wsign-compare]
wrap = phase < abs(phase_increment);
~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~
parasites/tides/generator.cc:978:18: error: reference to non-static member
function must be called; did you mean to call it with no arguments?
uint16_t x = mode == GENERATOR_MODE_AR ?
^~~~
()
parasites/tides/generator.cc:999:7: error: reference to non-static member
function must be called; did you mean to call it with no arguments?
mode == GENERATOR_MODE_AR ? pi << harm :
^~~~
()
parasites/tides/generator.cc:1000:7: error: reference to non-static member
function must be called; did you mean to call it with no arguments?
mode == GENERATOR_MODE_LOOPING ? pi * (harm + 1) :
^~~~
()
parasites/tides/generator.cc:1093:11: error: reference to non-static member
function must be called; did you mean to call it with no arguments?
if (mode == GENERATOR_MODE_AR) { // power of two harmonics
^~~~
()
parasites/tides/generator.cc:1099:18: error: reference to non-static member
function must be called; did you mean to call it with no arguments?
} else if (mode == GENERATOR_MODE_AD) { // odd harmonics
^~~~
()
parasites/tides/generator.cc:1271:16: warning: comparison of integers of
different signs: 'uint32_t' (aka 'unsigned int') and 'int'
[-Wsign-compare]
if (phase_ < abs(phase_increment_) &&
~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~
parasites/tides/generator.cc:1284:24: warning: comparison of integers of
different signs: 'uint32_t' (aka 'unsigned int') and 'int'
[-Wsign-compare]
if (delayed_phase_ < abs(delayed_phase_increment_)) {
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
parasites/tides/generator.cc:1360:16: warning: comparison of integers of
different signs: 'uint32_t' (aka 'unsigned int') and 'int'
[-Wsign-compare]
if (phase_ < abs(phase_increment_)) {
~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~
6 warnings and 5 errors generated.
make: *** [build/parasites/tides/generator.cc.o] Error 1

@JerrySievert
Copy link

the warnings (while true) can likely safely be ignored.

the errors look to start at parasites/tides/generator.cc line 978, just change mode == to mode() == on each of those and give it a run.

@JerrySievert
Copy link

looks like the change can be made with this repo only - I'll try compilation on macOS and linux, and if it fixes it on both, I'll open a PR.

@JerrySievert
Copy link

Failing in g++, but working in clang++ on Linux.

Looking closer, it looks like the inline keyword is being mucked with with some gcc specific overrides. I’ll take a closer look in a bit. If i can fix it and make it work by changing how inline overrides in clang++, i will, otherwise can put some #if statements in and give it correct behavior based on the compiler (which is what the inline overrides are doing, but this change would be more specific).

@JerrySievert
Copy link

aha, found the overall issue:

template<GeneratorMode mode>
void Generator::FillBufferHarmonic() {

which is aliasing an inline function. I have a 1 line fix that can be made into a PR, as well as go upstream, and should work on all platforms.

@lnikj
Copy link

lnikj commented Jan 8, 2019

I haven't tried editing the files yet Jerry and I hasten to add I am not a coder, just somebody who tries to build Mac versions of things when requested or when I find missing ones.

However, a thought ... the problem file in my attempt here is in the Mutable code not the Aepelzens no? The Mutable code builds fine when I do it with the Arable Instruments version.

So why would I need to edit the Mutable code?

Anyway, as I write, your latest has popped in and it looks like you have fixed it :)

@JerrySievert
Copy link

the issue comes down to this:

inline UiMode mode() { return mode_; }

is being defined at some point.

then later down the line, there's code in a template that looks like:

if (mode == SOMEVALUE)

at which the compiler is saying, hey! wait a minute! mode is a function that you told me to do something really special with, and now you want me to treat it as a variable?!!?. when in reality, the template this second bit of code lives in is overwriting what mode is. the solution? don't override mode, take the whole problem away.

the gcc/g++ compiler was more like, oh, you're probably doing something wrong here, but since you didn't tell me to warn you about anything I'm going to close my eyes and choose a solution and hope it's right (in this case, it happened to choose the right one, so it "worked"). clang/clang++, being a much smarter compiler, chose to expose the issue for the issue that it was, that someone is mucking with things, giving weird undefined behavior, and it will likely fail or seriously screw up with any minor tool change.

make a little more sense?

@JerrySievert
Copy link

ok, I've created a PR, up to @Aepelzen to accept or reject.

@lnikj
Copy link

lnikj commented Jan 8, 2019

A brilliant explanation sir. Chapeau!

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

No branches or pull requests

4 participants