Update custom solvers to fix LTO issue

[Tatsh]

The upstream solvers source has a few minor fixes for LTO. The difference is in this patch: https://github.com/gentoo/gentoo/pull/39303/files#diff-71e7b39bf8e46659833d0b157960dc63ab35c1d347cf411ff3d14cbfd62b3507

Could you make a new solvers-${sha}.tgz tarball with this patch applied?

This fixes the error when using strict LTO options: fgh_read.c:101:27: error: type of f2_HOL_ASL does not match original declaration [-Werror=lto-type-mismatch]

[svigerske]

After 4 years, it could be time to update the ASL version we use. Since a lot has changed, I've put this only into branch master for now and will then create a stable/2.1 for it after some testing.

[Tatsh]

For the new release, can you apply the dtoa.patch file?

[svigerske]

The tarballs we store in this github repo are usually just exact copies of a version from the AMPL source (Netlib or github/ampl/asl). We then apply our own patches in get.ASL.

It would be best to integrate dtoa.patch in the source at AMPL, but to argue for this, I would have to say why it was needed and I don't remember that. Do you have a case where applying this patch makes a difference?

[svigerske]

If I remove both patches, then that still seems to compile with gcc, clang, and the MS compiler on Linux/macOS/Windows without new warnings (https://github.com/coin-or/Cbc/pull/681/checks?sha=b192676a0d2281d48f2ea6a0209f00d55e8b4b10).

[Tatsh]

For GCC you have to enable -Werror=lto-type-mismatch. The fix in fgh_read.c is for those who want to have a 100% LTO compiled machine. This is the original bug: https://bugs.gentoo.org/943309

[svigerske]

Yes, I assume the fix for fgh_read.c is included in the new ASL source already.

But our dtoa.patch that adds a #include <stddef.h> to dtoa1.c is not in there, and I don't remember and cannot reproduce where this was necessary for. So this patch will go away in stable/2.1.

The mingw.patch was probably there to avoid some multiple defined _matherr symbol on Windows. According to https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/matherr?view=msvc-170, it is however allowed to overwrite this function. So I'll remove this patch as well for now. If the problem for which the patch was added (in 2016) comes back, we can rather add a -DNO_matherr in that situation. For GCC/clang, ASL source itself already ensured NO_matherr to be defined.

[Tatsh]

Okay, I tested here as well and it looks like dtoa.patch is not required (no warning about implicit declaration appeared).

With the new solvers being used here, there is only a minor issue left for LTO:

solvers/fgh_read.c:101:15: error: function 'f2_HOL_ASL' redeclared as variable
  101 |  extern char* f_OPHOL;
      |               ^
solvers/rops2.c:1019:1: note: previously declared here
 1019 | f_OPHOL(expr *e A_ASL)
      | ^
lto1: fatal error: errors during merging of translation units
compilation terminated.

Would you be willing to make a new tarball of solvers with this change?

fgh_read.c line 101 change:

extern char* f_OPHOL;

to

extern char* f_OPHOL ANSI((expr *e));

To see this error build with C flag -flto=auto enabled.

[svigerske]

It wouldn't be version 20241108 anymore, if I start adding patches there.

I think it would be best when you create a pull request at https://github.com/ampl/asl, and then we make a new tarball here when they have a new version out.