build of 0.5.1 fails

[rofl0r]

src/asm/parser.y:531.1-11: invalid directive: ‘%precedence’
src/asm/parser.y:671.19-24: invalid directive: ‘%empty’
src/asm/parser.y:776.19-24: invalid directive: ‘%empty’
src/asm/parser.y:807.19-24: invalid directive: ‘%empty’
src/asm/parser.y:870.19-24: invalid directive: ‘%empty’  
src/asm/parser.y:951.19-24: invalid directive: ‘%empty’
src/asm/parser.y:1069.19-24: invalid directive: ‘%empty’
src/asm/parser.y:1606.19-24: invalid directive: ‘%empty’
src/asm/parser.y:1630.19-24: invalid directive: ‘%empty’
src/asm/parser.y:1645.19-24: invalid directive: ‘%empty’
src/asm/parser.y:1656.19-24: invalid directive: ‘%empty’

bison --version
bison (GNU Bison) 2.6.2
Written by Robert Corbett and Richard Stallman.

my bison version compiles everything else (an entire linux distro) fine - rgbds is the only program using these bleeding egde features.

[ISSOtm]

RGBDS 0.5.1+ requires Bison ≥ 3.0 (from 2013), which we consider reasonable.
2.6.2 (from 2012, only a couple of versions before 3.0) is, AIUI, the default on macOS (being the last GPLv2 version); we recommend using the version from Homebrew.

Is that fine with you?

[rofl0r]

no, that's why i opened pr #923 - there's absolutely no reason for these changes, and you slipped it in like a trojan horse under an innocent title - "enable bison warnings"

[ISSOtm]

The reason for these changes is to simplify the code (%precedence and then-redundant directive removal), and make modifications more reliable (/* empty */ does not guard against a forgotten |, but %empty does). There is reason for these changes.

It was not "slipped in", those changes were in the spirit of the commit to improve reliability. These specific changes are not enabling warnings per se (as -Wall would do), but they are enabling better error reporting as described above, so I consider the topicality correct. (And even if it wasn't the case, mistakes happen occasionally.)

I was asking if "this is fine with you" to learn why using Bison ≥ 3.0 may not feasible or practical for you, and hopefully reach a satisfactory compromise between our goals and yours.

[daid]

Insert xkcd workflow.

[rofl0r]

I was asking if "this is fine with you" to learn why using Bison ≥ 3.0 may not feasible or practical for you, and hopefully reach a satisfactory compromise between our goals and yours.

flex and bison are quite buggy codebases and it's difficult to find versions that work reliable across all packages in my distro, without going for day-long patch orgies. 2.6.2 works for all other packages so i'd like to avoid bumping it just for the sake of a single package.

[ISSOtm]

Alright. I have to run a couple errands, so I'll handle this in a few hours.

I see your use case, and I understand that you may want not to patch RGBDS (though I know a couple of downstream packagers, such as the BSDs, do it for us for other minor reasons).

I don't want to hard-revert those changes due to the benefits they provide, but maybe we can look into a sed invocation, if Bison < 3.0 is detected, that would replace %empty with the comments, etc. Can that work out?

[rofl0r]

if Bison < 3.0 is detected, that would replace %empty with the comments, etc. Can that work out?

that could work for %empty, though i guess not for %precedence - as your commit removed the %left operator from everywhere else.

what could work is if parse.y is generated from a template (parse.y.in) using e.g. a preprocessor pass or similar on it.

[Rangi42]

I'm personally against making the parser file and the build process more complicated for the sake of supporting an eight-year-old bison version. macOS users should be used to upgrading various outdated built-in programs (IIRC they stick with some very old Linux utilities for license reasons), and Linux/BSD/etc users are capable of either patching parser.y to avoid bison 3 features, or patching the Makefile to use a local copy of bison 3. (I'm used to doing that myself with different rgbds versions for some old assembly projects.)

However, if we do end up adding a sed patching step, this could work:

• Update PR Make parser.y again compatible with bison 2.6 (the default on macOS) #923 to replace the "%left"s that it adds with "%bison2_left"
• In the Makefile, if the bison version is 3, use sed to remove all lines starting with "%bison2_left"
• If the bison version is 2, replace all "%bison2_left" with "%left", replace all "%precedence" with "%left", and remove all "%empty"

That would isolate most of the added complexity in the Makefile (or better, in a script called by the Makefile), and the extra "%bison2_left"s in parser.y would at least be self-explanatory.

[rofl0r]

the reason why i didn't follow up on this is because "how to do it for windows/cmake"

[AntonioND]

If this is such an issue for you, you can build a new version of bison and instead of installing it in your system you can just override the one used in the build process:

make BISON=/my/path/to/bison ...

[rofl0r]

you can build a new version of bison and instead of installing it in your system you can just override the one used in the build process:

for a distro author, as opposed to a distro user, this is not as easy as it sounds.
first, i need to consider cross-compilation. that means the "throw-away" bison needs to be built for the host platform, rather than the target platform. secondly, building a given version X of something, rather than what's already supported, might require a whole series of patching to fix build and runtime issues (if you look at the sources of random debian packages, you will notice that most of them having multiple, if not dozens of patches). this effectively means investing the same amount of work than updating the package to begin with. this is especially demanding if you use a non-mainstream platform, for example a linux distro based on musl libc.
the probably most notorious case where every different version requires a multitude of different patches is llvm, which makes bootstrapping rust from source an adventure, because you need to bnuild like 20 different llvm versions in the process, each one requiring a huge investment to make it build and fix bugs again that are long fixed in latest version.

[AntonioND]

Which distro is this one that is stuck in such an old version of everything, out of curiosity? It doesn't sound practical at all to use it.

[rofl0r]

my distro is called sabotage linux. it's not "stuck in old versions of everything" - for example it has latest LTS kernel, latest openssh, etc - but i prefer to invest work into things that are important, and stability over features.
blog is here https://sabotage-linux.neocities.org/blog/ .

[Rangi42]

Then why not stick with rgbds 0.5.0? Sure it has known bugs and missing features, but so do bison 2 and other old packages. And it's not like a Game Boy assembler is a necessary component for a Linux distro. (Especially since Sabotage Linux "refuses to add Rust", and the rgbds ecosystem is increasingly using Rust, e.g. rgbobj and rsgbgfx.)

[ISSOtm]

Also a planned RIIR of RGBDS, since a rewrite is more or less needed due to some of RGBASM's architectural decisions. It's a long-term plan, but still worth considering.

[rofl0r]

Then why not stick with rgbds 0.5.0

i think you're misunderstanding my motivation. i already have a working patch for 0.5.1, which will be forward-ported as needed. i opened these issues to raise awareness of the issues caused by using a seemingly innocent new feature of some dependency. in the end, it's of course your choice whether you prefer breaking backwards compatibility for the gain of using a new keyword over comments.

rgbds ecosystem is increasingly using Rust
planned RIIR of RGBDS

that's quite unfortunate. i'd suggest the following the following literature to understand the long-term effects of this move:

https://drewdevault.com/2019/03/25/Rust-is-not-a-good-C-replacement.html
https://ariadne.space/2021/09/16/the-long-term-consequences-of-maintainers-actions/
https://blog.adelielinux.org/2017/11/18/our-official-stance-on-the-rust-programming-language/

[Rangi42]

I don't know how far off a Rust rewrite is (that would be @ISSOtm 's project) but I'm considering starting a more gradual transition to C++, for the sake of using some of its features (in particular to unblock #885, but I expect RAII idioms will also make for cleaner code).

Not speaking for the other maintainers, but I don't think being written in C adds any inherent value to rgbds, besides allowing well-optimized performance of native code (same as C++, Rust, Zig, Haskell, and many other languages) and being supported on "enough" platforms (anything with an LLVM backend is sufficiently portable here).

Drew DeVault may think that "concurrency is generally a bad thing" and that "Yes, Rust is more safe. I don’t really care." but in practice C has been a cause of rgbds issues (use-after-free, buffer overflows, null-terminated strings, having to manually malloc/free, undefined behavior, etc) not a solution to them (yes, C has a spec and only "0.73 new features per year"; I don't really care). (Even he is aware enough of C's issues to be building a new systems language.)

[aaaaaa123456789]

I think I've spoken loudly enough against Rust in every venue I've been afforded to not be mistaken for a Rust supporter, but I'll go ahead and say this: rewriting RGBDS in C++, Rust, or any other similar language is not inherently harmful.

I'm a user, not a maintainer. I've contributed very little code to RGBDS. And from a user's perspective, what matters is that the software (1) works, and works well, (2) is easy to build and update, and (3) can be installed on any platform I use. (1) and (3) are equally true of all those languages. (2) is fine for C++, but more problematic for Rust, because Rust loves being the odd one out and using a build process that is different from the rest of the universe (including fetching packages from the Internet as part of the process!), but I assume skilled Rust devs could wrap this into a build process that is compatible with the world and doesn't require (or use) an active connection to the Internet.
The move means I won't be contributing in the future, sure — I don't know Rust and don't have any interest in learning it (I've tried enough times to realize I simply don't enjoy the language). But I'm not a contributor right now, so that has no impact. The same goes for literally any other non-contributing user out there. As a user, the language the tool is written in is irrelevant.

So I'd fully support moving to a language the maintainers are comfortable writing in, whatever that may be, as long as the conditions I mentioned above are preserved. In the case of Rust, that probably means writing a simple makefile that wraps rustc (or cargo if you must) and doesn't trigger online package fetches unless you tell it to.

[AntonioND]

I think that a transition to C++ is far more realistic (and so, more likely to actually happen) than a complete rewrite in Rust. It would also keep the build system simple, which is crucial, in my opinion.

I kinda hate the kind of things that some people do with C++, though.

[Rangi42]

I don't plan on doing anything weird with class inheritance or templates (probably no need for templates at all, unless we have like three+ functions which are identical except for types). Feel free to critique PRs for being too C++ featureful if/when I work on that. :P

[rofl0r]

in practice C has been a cause of rgbds issues (use-after-free, buffer overflows, null-terminated strings, having to manually malloc/free, undefined behavior, etc) not a solution to them

i doubt it's been C causing those issues, but programmers using it incorrectly...

transition to C++, for the sake of using some of its features (in particular to unblock #885, but I expect RAII idioms will also make for cleaner code)

i see you already refactored the code to use a struct string, which seems like a good way forward. what i glanced from the PR it seems almost complete and there's probably not a lot left to change - a rewrite using C++ would be a whole lot more work.
coincidentally, i've been writing a similar string library in the past: https://github.com/rofl0r/libulz/blob/master/include/stringptr.h - maybe you can draw some inspiration from it.

btw, here's what linus torvalds thinks of C++: http://harmful.cat-v.org/software/c++/linus

[Rangi42]

Both C++ and Linus were very different in 2007 than in 2021. By now not even C++ but Rust is even making its way into Linux, at least for device drivers.

If you want to complete that PR I'd really appreciate it; it's not like I'm going to be tackling it in the next week or so. Even so, it's basically a tedious reimplementation of what C++ (or any language with a built-in memory-managed (counting RAII) {ptr, length, capacity} type, especially for strings) would provide by default. (And we're not even shooting for the small-string-optimization that's standard in C++, Rust, etc.)

Also the first file I checked in strlib, containsChar.c, could have just been return strchr(str, what) != NULL. (Which doesn't say much good about C's built-in "string" (aka pointer to some bytes which we hope end in a '\0') handling functions either.)

[ISSOtm]

i see you already refactored the code to use a struct string, which seems like a good way forward. what i glanced from the PR it seems almost complete and there's probably not a lot left to change - [...]

The problem is that what there is to change is making it work with the entirety of hashmap.c, which mind you, is already used elsewhere in the codebase (RGBLINK, to be precise).

[AntonioND]

btw, here's what linus torvalds thinks of C++: http://harmful.cat-v.org/software/c++/linus

C++ is a bad language for a kernel for the simple fact that the language does memory management without your control. For example, I was once writing a firmware for a microcontroller, and someone decided to start adding C++ stuff. It took myself and another guy a couple of hours to convince him that his std:pair statically allocated array was pulling malloc and company into the firmware. So yeah, I'm not a fan of C++ fans.

However, If you just use C with classes and maybe with operator overloading and things like that... the result tends to be okay. Just avoid the C++ standard library... and almost every other library. I would use std::string and little else.

[rofl0r]

Also the first file I checked in strlib, containsChar.c, could have just been return strchr(str, what) != NULL. (Which doesn't say much good about C's built-in "string" (aka pointer to some bytes which we hope end in a '\0') handling functions either.)

you're looking in the wrong place, the string+size struct stuff is provided by src/stringptr - and the purpose of strlib is to pull in as little of libc as possible, to keep static binaries small (iirc the only libc funcs called are memcpy and malloc* - and the latter only if a *_new func is used)

However, If you just use C with classes and maybe with operator overloading and things like that... the result tends to be okay. Just avoid the C++ standard library... and almost every other library. I would use std::string and little else.

that's true, but once C++ is being used, you're automatically attracting C++ "wizards", which will start opening PRs using their favorite feature subset... just as linus points out in that article.

[ISSOtm]

Sure, but we can simply review out unnecessarily complex code from contributions. (Whether you trust us not to write such code ourselves is left as an exercise to the reader.)

As for the possible performance impact of such a decision, we do not have any performance regression tests yet because we haven't figured out how to do those, but please feel free to chime in to #653 if you have any ideas.