Compiling Wren: some math, strtol and %g format string issues

[alisonatwork]

Inspired by @ahgamut's work with Lua on #61 I thought I'd try compile Wren just for fun. It worked pretty well with just a few minor changes. I cloned here with my changes on the cosmopolitan branch: https://github.com/alisonatwork/wren/tree/cosmopolitan

Changes required were:

• some small Makefile hacks to compile the static lib and test suite with the Cosmopolitan flags
• bunch of dummy header files to fake out #include <stdio.h> and so on
  • Since this was also required for Lua and everything else I have tried to compile, I wonder if it might be nice to provide these stubs as a package somewhere? Or is there another solution?
• missing math function modf
  • I inlined the one from musl libc
• a few function name clashes in the test suites
• workarounds for the %g format string problem
  • I used if (trunc(f) == f) sprintf(s, "%ld", (long) f) to at least get the int-ish doubles printing as expected and %f for best-effort on the floaty doubles

With these changes, 797 tests pass and 42 fail. Almost all of these are format string related like this:

      Expected output "0.123" on line 5 and got "0.123000".
      Expected output "0.3" on line 6 and got "0.300000".
      Expected output "-0.123" on line 7 and got "-0.123000".

But there are a few suspicious ones that I still need to investigate.

For now, the great news is that this language almost completely works!

The bug reports:

1. modf missing
2. %g format string

Requests for input:

1. Anyone have a good idea how we can hook the compiler or somehow fake out angle-bracket includes on-demand? Is there a better alternative to providing stub headers or pushing in #ifdef __COSMOPOLITAN__ everywhere?

[jart]

That's great news! You now have float formatting and modf. Name clashes are in scope since there's still a lot of technical debt to pay off in terms of removing functions I whipped up on the fly for the sake of moving quickly.

In terms of all the posix headers, what I'm hoping we can get away with is a shell script example code snippet that looks something like this:

mkdir include
for x in string stdio stdlib ...; do
  touch include/$x.h
done

In which case the -include cosmopolitan.h flag takes care of the rest. I kind of want the easy way to be the default way. It's going to break codebases that do things like define their own clashing symbols where they assume things aren't being defined based on the subset of stdinc headers they're using. But I'm kind of hoping those kinds of codebases are rare, and we can put off the need to write a perfectly conforming set of headers for as long as possible.

Let me know what else you need!

[alisonatwork]

This is really awesome, thanks! With the new float formatting and the math functions I was able to rebuild Wren with only a couple of makefile changes and name clashes. We're down to only 15 tests failing. I'll take a look at exactly why they're failing a bit later on and report back.

The touching of empty headers is the approach I was taking, and it seems to work well for most projects. Just a little script to touch the standard ISO C set plus a few of the POSIX ones should get people a long way.

[alisonatwork]

Next problem was formatting hex numbers, which should be fixed in the PR I just opened. Now we are down to just 3 errors in the Wren test suite.

[jart]

Nice. I like this Wren language. Can't wait to see it up and running. Thanks again for the PR. I've now configured Travis to provide build/test feedback on any future ones you send. The latency is two minutes, which is pretty good, if we consider that it needs to compile and run about 400 executables.

[alisonatwork]

It's all looking pretty good now with the changes from #107 and #110, just two more errors to pin down:

$ python3 util/test.py
FAIL: test/language/number/literal_too_large.wren
      Missing expected error on line 1.
      Expected return code 65 and got 0. Stderr:


FAIL: test/core/number/from_string_too_large.wren
      Expected runtime error "Number literal is too large." and got none.
      Expected return code 70 and got 0. Stderr:

Both of these are to do with parsing a ridiculously large number. I'm still not convinced my changes for strtoimax work perfectly, but I'll keep digging.

I don't think this language is worthy of being included in the distro, since it's a fair bit bigger (and a lot less popular) than Lua, but I agree that it's kind of nice to see a clean OO scripting language compile pretty simply against Cosmopolitan. Their wren-cli REPL does a bunch of native OS stuff that makes compiling it a bit trickier, but just getting the VM and standard library embeddable is neat.

[jart]

One thing worth noting is that other libraries define intmax as long but we define it as the standard says which is the largest integer type supported by the compiler. I'll take a look too and see if I can further test the implementation a bit.

[jart]

What you did appears correct so far. It helps if you encode two's complement bane in its unreadable decimal form:

  errno = 0;
  EXPECT_EQ(0x7fffffffffffffff, strtol("0x8000000000000000", NULL, 0));
  EXPECT_EQ(ERANGE, errno);
  errno = 0;
  EXPECT_EQ(0x8000000000000000, strtol("-0x8000000000000001", NULL, 0));
  EXPECT_EQ(ERANGE, errno);
  errno = 0;
  EXPECT_EQ(0x8000000000000001, strtol("-9223372036854775807", NULL, 0));
  EXPECT_EQ(0, errno);
  errno = 0;
  EXPECT_EQ(0x8000000000000000, strtol("-9223372036854775808", NULL, 0));
  EXPECT_EQ(0, errno);

I was a little unhappy when I first learned about that. I came to the conclusion that negative numbers and hexadecimal are dangerous when put the two together and I've sought to avoid that ever since.

[alisonatwork]

Hmm, this is interesting, it seems that strtod behaves different in David Gay's gdtoa than in musl libc.

For this file:

#ifdef NO_COSMO
#include <errno.h>
#include <limits.h>
#include <stdlib.h>
#include <stdio.h>
#endif
int main() {
 errno = 0;
 double d = strtod("9007199254740992", NULL);
 printf("d = %g errno = %d\n", d, errno);
 double d2 = strtod("999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999", NULL);
 printf("d2 = %g errno = %d\n", d2, errno);
}

Compiled with and without Cosmopolitan:

# ./a.out
d = 9.0072e+15 errno = 0
d2 = inf errno = 34
# ./b.com.dbg
d = 9.0072e+15 errno = 0
d2 = inf errno = 0

So, the outstanding error of Wren seems to be strtod-related.

[jart]

Right now the build is configured with -fno-math-errno but I can turn that off if you need it.

[alisonatwork]

Perfect! Removing -fno-math-errno gets all of the Wren tests passing.

I wonder if we should #ifndef __NO_MATH_ERRNO__ guard the errno setting in strtol family and strtoimax too? That would at least make it consistent with the behavior of gdtoa. Although, to be honest, I think it's a bit more mysterious for gdtoa to disable errno completely in this instance. GCC docs say:

Do not set errno after calling math functions that are executed with a single instruction, e.g., sqrt. A program that relies on IEEE exceptions for math error handling may want to use this flag for speed while maintaining IEEE arithmetic compatibility.

It'd have to be a pretty awesome strto* implementation to execute in a single instruction!

Whatever we decide to do with the errno, I think we can call this ticket pretty much done, since either it will compile out of the box, or it will compile with math-errno enabled.

With regard to the name clashes, these were with standard functions remove(3) and write(2) so I don't think it's a problem we can fix in Cosmopolitan. They don't appear as errors in the original Wren build because those tests don't include stdio.h or unistd.h, but our -include cosmopolitan.h has all of them. I think they should accept an upstream PR to tweak that since it's only the tests anyway.

[ruby0x1]

Thanks for the work on this btw, I've been following Cosmopolitan and it's nice to see Wren making it's way there relatively painlessly! ✨

[jart]

Thank you for pointing out the GCC doc. It's pretty clear in that case the flag is intended to let the compiler emit fsqrt and sqrtss instructions. So I've just removed those guards.

Needing to not clash with standard library functions is a calculated tradeoff with the amalgamation header. I believe it should be possible to workaround in your build config, possibly by defining a wrapper header like this:

/* cosmopolitan2.h */
#define remove __redacted_remove
#define write __redacted_write
#include "cosmopolitan.h"
#undef remove
#undef write

Nice to meet you @ruby0x1. I hope Cosmopolitan proves beneficial to your work and helps it reach a broader audience. Using Cosmopolitan with Wren is painless as of today and we can thank @alisonatwork for that. For the past few years this project has been like a model train set for me, that many people have loved and encouraged so much that we're rapidly turning it into a high-speed rail. I appreciate all the support wrinkling out bugs and I hope you continue to follow its future going forward.

[shmup]

@alisonatwork howdy! I wondered if you happened to transfer this repo elsewhere, or simply nuked it?

If anyone can spot a fork, I'm interested ^__^ cc: @ruby0x1

[alisonatwork]

Hi @shmup I don't have my Wren branch any more that I built with Cosmopolitan, but the reason I nuked it is because it was pretty trivial by the time this issue was closed. With the changes that were made in both projects, the build ran pretty much clean. By the sounds of it Cosmopolitan has come a long way in the past year, so it should be even easier now.

If I recall the two main things I needed to do were:

1. create a hierarchy of empty stub header files to satisfy includes
2. add "build on Cosmopolitan" tweaks to the Makefiles in projects/make/ directory of Wren

Both are standard things required to get any project compiling with Cosmopolitan. If the Wren maintainer is open to the idea, it should be viable to add a whole separate target in projects/ directory specifically for building on Cosmopolitan, with the appropriate changes made to the Makefile and perhaps a little bootstrapper to generate the include stubs.

[alisonatwork]

Ah, just looking back through the Wren source code, I remember why I stopped work on my branch. The makefiles in projects/make/ are actually generated by a tool called Premake, so in order to add a Cosmopolitan build target in a clean way (i.e. not by manually patching the existing makefiles), it would ideally need to be supported as a platform in Premake. That would be a cool project to do, because theoretically it could enable multiple projects that use Premake to add a Cosmopolitan build, but it's a rabbit hole that was too deep for what I had time for. Maybe someone in the Cosmopolitan community is keen to take it on now, though? However, if you just want a version of Wren that compiles so you can static link it into your own project, or if you want to vendor the code in your own project, that should work pretty easily right now with the usual tweaks.

[pkulchenko]

That would be a cool project to do, because theoretically it could enable multiple projects that use Premake to add a Cosmopolitan build, but it's a rabbit hole that was too deep for what I had time for.

FWIW, Premake is a nice tool and uses Lua for its scripts.