search

TALP-UPC / FreeLing

FreeLing project source code
Other
252 stars 96 forks source link

Build failure with Boost 1.76 on macOS Mojave and Big Sur #114

Closed carlocab closed 3 years ago

carlocab commented 3 years ago

Building FreeLing on macOS Mojave (10.14.6) and Big Sur (11.3.1) with Boost 1.76 fails with the following error:

/tmp/freeling-20210524-62368-1exwnh0/FreeLing-4.2/src/include/freeling/morfo/smoothingLD.h:129:73: error: use of undeclared identifier 'log'; did you mean 'long'?

Oddly enough, this does not fail on macOS Catalina (10.15.7).

This seems straightforward enough to fix with

#include <cmath>
using std::log;

but I wanted to check in before trying to apply that fix in case this has consequences I'm not aware of.

Build logs available at https://github.com/Homebrew/homebrew-core/actions/runs/870966679. The error on (Intel) Big Sur starts at https://github.com/Homebrew/homebrew-core/runs/2654992071?check_suite_focus=true#step:7:2359.

See also https://github.com/Homebrew/homebrew-core/pull/75459.

carlocab commented 3 years ago

Though, applying the fix I suggest above leads to another missing #include:

/tmp/freeling-20210524-61311-n18juv/FreeLing-4.2/src/libfreeling/csr_kb.cc:157:19: error: use of undeclared identifier 'fabs'
        change += fabs(ranks[NEXT][v] - ranks[CURRENT][v]);
                  ^

https://github.com/Homebrew/homebrew-core/runs/2661832770?check_suite_focus=true#step:7:2221

lluisp commented 3 years ago

You are right, that should fix it (actually, it is a bit surprising it works without it...) I added also "using std:fabs", hope it works now I changed it in master, if you want to pull

carlocab commented 3 years ago

Thanks for the fix! I am curious about https://github.com/TALP-UPC/FreeLing/commit/aaf733521a329d26aa71dd0e34da340a8b4a1291, since the error about the undeclared identifier fabs comes from src/libfreeling/csr_kb.cc, but I don't see it #include-ing smoothingLD.h anywhere.

Am I just confused?

lluisp commented 3 years ago

Ups, my fault. I assumed the problem was in smoothingLD as well, since it also uses fabs...

But... you mean the fix solved that too? that is weird... :S

carlocab commented 3 years ago

I haven't tested the using std::fabs fix yet. It's chugging through CI now, which takes a while (since it's part of a boost version bump which tests all these other boost dependencies).

carlocab commented 3 years ago

Actually, I'm also confused by the new error I mention in https://github.com/TALP-UPC/FreeLing/issues/114#issuecomment-847631999.

In src/libfreeling/csr_kb.cc, you have

https://github.com/TALP-UPC/FreeLing/blob/0dcd3373433398123da602eddd8c0d351f393826/src/libfreeling/csr_kb.cc#L6

and

https://github.com/TALP-UPC/FreeLing/blob/0dcd3373433398123da602eddd8c0d351f393826/src/libfreeling/csr_kb.cc#L12

In src/include/freeling/morfo/util.h,

https://github.com/TALP-UPC/FreeLing/blob/0dcd3373433398123da602eddd8c0d351f393826/src/include/freeling/morfo/traces.h#L35

which defines std::fabs.

carlocab commented 3 years ago

Ok, one of the CI nodes finished (the ARM node is a lot faster than the others), and it seems (I guess unsurprisingly) that https://github.com/TALP-UPC/FreeLing/commit/aaf733521a329d26aa71dd0e34da340a8b4a1291 does not fix the undeclared identifier error for fabs.

As for https://github.com/TALP-UPC/FreeLing/issues/114#issuecomment-847723544, I just misread cppreference. cstdlib defines only abs, but not fabs. For fabs we need cmath.

lluisp commented 3 years ago

I guess you are using clang, and maybe it uses different defaults than g++ ...

I included cmath wherever fabs is used... check if it works now.

carlocab commented 3 years ago

Yes, we do build with Clang.

Your new patches seem to have done the trick, at least on ARM. Let me just see how the build does on the other CI nodes (should be done at some point tonight), but I expect to be able to close this issue soon.

Thanks!

carlocab commented 3 years ago

Yep, can confirm that your patches fixed everything. Thanks for this!