lcalc: fix clang-19 build

[paparodeo]

https://gitlab.com/sagemath/lcalc/-/issues/16

add -D_GLIBCXX_COMPLEX -D_LIBCPP_COMPLEX -D_LIBCPP___FWD_COMPLEX_H to prevent including system and use the vendored copy.

https://hydra.nixos.org/build/282766851

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[collares]

Scarily, the code includes a full copy of complex.h from GCC 3.5! Do you happen to know if the code was actually using Lcomplex.h's definition of complex instead of the standard library's? I think it's worth checking, but if you don't have the time I can do it later.

[paparodeo]

I believe the vendored one is getting used as just including <complex> results in a bunch of errors

invalid operands to binary expression ('int' and 'Complex' (aka 'complex<double>')

with clang-19.

probably be better to fix the errors and use <complex> tho.

[collares]

Thanks for checking! While I agree that using <complex> would be better as a final goal, I think it would be better to have upstream review of such a big change. I will try to think if there are even less invasive alternatives than your solution.

[paparodeo]

Thanks for checking! While I agree that using <complex> would be better as a final goal, I think it would be better to have upstream review of such a big change. I will try to think if there are even less invasive alternatives than your solution.

for clang-19 I initially just added a compiler flag -D_LIBCPP___FWD_COMPLEX_H which prevented the conflicting include file from getting parsed.

https://github.com/llvm/llvm-project/blob/main/libcxx/include/__fwd/complex.h

[collares]

Hmm, that's a good point! Does it build with -D_GLIBCXX_COMPLEX -D_LIBCPP_COMPLEX (the first one is the libstdc++ equivalent)? If it does, I think we can keep it as a workaround for now while your upstream fix is reviewed (in which case, please add a comment above the workaround linking to your upstream MR, as you currently do with the patch).

[paparodeo]

this builds for both platforms:

  env.NIX_CFLAGS_COMPILE = toString [
    "-D_GLIBCXX_COMPLEX"
    "-D_LIBCPP_COMPLEX"
    "-D_LIBCPP___FWD_COMPLEX_H"
  ];

I could make the conditional on stdenv.cc.libcxx != null or just leave them both. I don't think it will hurt to just leave them all in.

[collares]

I don't think it needs to be set conditionally, leaving all is fine. Do you know if the third one is needed, given the second?

Thanks so much for the workaround and for the upstream MR!

[paparodeo]

I don't think it needs to be set conditionally, leaving all is fine. Do you know if the third one is needed, given the second?

Thanks so much for the workaround and for the upstream MR!

-D_LIBCPP___FWD_COMPLEX_H is required to build. -D_LIBCPP_COMPLEX could be dropped but seems fine to leave it in for safety.

[collares]

-D_LIBCPP___FWD_COMPLEX_H is required to build.

I don't really mind having the third line, but just to be sure I know how to test Clang fixes: I replaced stdenv by clangStdenv in the derivation and it built with just "-D_GLIBCXX_COMPLEX -D_LIBCPP_COMPLEX" on Linux. Should I be testing differently?

[paparodeo]

-D_LIBCPP___FWD_COMPLEX_H is required to build.

I don't really mind having the third line, but just to be sure I know how to test Clang fixes: I replaced stdenv by clangStdenv in the derivation and it built with just "-D_GLIBCXX_COMPLEX -D_LIBCPP_COMPLEX" on Linux. Should I be testing differently?

that just pulls in libstd++ you need libc++ which is libcxxStdenv

[collares]

Got it, thanks!

[paparodeo]

so python3Packages.sagelib includes Lcomplex.h and fails to compile. so this isn't going to work for building sage with libcxx-19.

the original PR that used https://gitlab.com/sagemath/lcalc/-/issues/16 works tho.

(FWIW I've gotten sage to build on darwin now after a few more changes and some fixes to gcc darwin build)

[paparodeo]

so can also just replace Lcomplex.h with <complex> and add operators with complex<double> and an int. nothing seems to be doing anything with a complex and a long or long long (I added operators with no definitions and didn't get a link errors.

inline Complex operator* (const Complex &l, int r) { return l  * double(r); }
inline Complex operator/ (const Complex &l, int r) { return l  / double(r); }
inline Complex operator+ (const Complex &l, int r) { return l  + double(r); }
inline Complex operator- (const Complex &l, int r) { return l  - double(r); }
inline bool    operator==(const Complex &l, int r) { return l == double(r); }
inline bool    operator!=(const Complex &l, int r) { return l != double(r); }

inline Complex operator*(int l, const Complex &r) { return r * l; }
inline Complex operator+(int l, const Complex &r) { return r + l; }

inline Complex operator/(int l, const Complex &r) { return double(l) / r; }
inline Complex operator-(int l, const Complex &r) { return double(l) - r; }

[collares]

Ok, let's go with 166b3cd then. Sorry for not testing Sage!

[paparodeo]

it only breaks with clang / libc++ so gcc / libstd++ is fine. will setup a PR in a little bit.

[collares]

Ah, good to know. If it's clang only I will wait for your PR instead of reverting now.