Feature request: #pragma once or include_once equivalent

[sukus21]

Include guards are a common problem in languages with include files. In some C/C++ compilers, the problem is remedied by supporting the #pragma once precompiler directive. I would like to suggest that RGBDS implements a feature similar to this. I will be reffering to it as pragma once throughout this issue, even though I don't necessarily think that should be what it's called in RGBDS (it could be called something like INCGUARD?).

My assumption is that RGBASM keeps track of the name of the file it is currently processing. There could be a table somewhere keeping track of files that have been #pragma once'd. Whenever the directive is hit, check if the current file is in the table. If it is not in the table, add it and keep processing it. If it is in the table, don't include the rest of the files content from that point forward.

Keep in mind I have no clue how possible it is to implement this. I am not familiar with the RGBDS codebase, and my attempts to build it by hand have been futile so far. I hope this will be considered :)

[ISSOtm]

This is in theory possible, but #pragma once has many problems, e.g. with symlinks and hard links.

That said, it's used in C because forward-decls are often necessary, and thus headers can end up referring to each other. This hardly ever happens in assembly AFAICT, so do you have a particular counter-example in mind?

[Rangi42]

We could also go for an INCLUDEONCE statement, which would avoid edge cases like "what if pragma once occurs in the middle of a file" or "can pragma once be wrapped in an if as long as nothing else 'meaningful' happens before it".

Prior art: PHP (include_once), Objective-C (#import), and PL/I (%XINCLUDE).

[sukus21]

do you have a particular counter-example in mind?

Yes, I do have an example. I use include files to define constants and macros, that I need to reach from multiple files. This creates the same problem as exists in C, where the functions (in this case, the macros or constants) have to be forward declared

[sukus21]

include_once is a wonderful idea, that is so much better!

[Rangi42]

Out of two popular included "libraries", hardware.inc has an include guard and structs.inc does not. The pret disassembly projects which I'm familiar with also do not have a double-include problem, since they -Preinclude all necessary constants and macros at once.

I'd like to get more gbdev users' feedback on this proposal, see how in-demand this would be.

[Rangi42]

Re: symlinks and hard links, if we added an include_once directive, I'd expect it to be based on the exact filename, before the file is even located/opened/read. So you could even do include_once "foo.inc" and then include_once "././foo.inc" to include foo.inc twice.

[ISSOtm]

Yes, I do have an example. I use include files to define constants and macros, that I need to reach from multiple files. This creates the same problem as exists in C, where the functions (in this case, the macros or constants) have to be forward declared

But such files only need each .inc once, so I still don't see the use case.

(And, constants can be exported in many cases, too, FWIW.)

[sukus21]

But such files only need each .inc once, so I still don't see the use case.

Yes, you are right, that is why I want the include guard. I have had situations where I define a macro in one .inc file, then use it in a specific way in another .inc file. In the .asm file, I need the functionality from both. And I would personally rather include both files, so it is clear that I use both to the reader of the file (often just myself).

I guess it comes down to how I like to structure my projects. I would rather explicitly include a header, than have it implicitly included by another header. The existance of INCLUDE_ONCE woulen't interfere with the behaviour of the regular INCLUDE, so no behaviour is taken away.

[sukus21]

...and my attempts to build it by hand have been futile so far.

Guess what hurdle I just managed to overcome thanks to WSL!
I have implented a prototype of what I think the INCLUDE_ONCE directive should look like in #1481.
I am not used to writing C/C++, and have no prior experience with Bison, and couldn't get pkg-config or clang-format to work properly, but did manage to run (most of) the tests, by hacking a little in the run-tests.sh script.

I look forward to hearing your feedback!

[ISSOtm]

Could you link to a project that would benefit from it, please?

We must balance two things: how useful the feature would be to other projects, and the cost for us to maintain it. I do appreciate the effort you've put into the PR (that you've set up a dev env, added some testing...), but it's a burden for the maintainers (the more code we have, the more has potential to break, and the more we have to update when making some internal changes, for example).

This is why we want every new feature to clear an (arguably fuzzy) “minimum added value” bar. If you can show us a kind of project that suffers from the lack of this feature, that would help us gauge that further.

Thanks!

[Rangi42]

Apparently esprit could benefit; some of its includes have C-style header guards.

(Of course, the whole project's conventions for including things could be refactored to avoid that need, e.g. by copying pokecrystal or gb-starter-kit and having one file responsible for including everything you might need; but that would be a reason to close this feature request outright as "can technically be worked around".)

[Rangi42]

Also if pokecrystal ends up doing something like pret/pokecrystal#631, then I expect INCLUDE_ONCE would be useful. It would let us keep constants split up into many organized .inc files, let them include other constants/macros necessary for their own definitions, and let end users write custom map scripts more easily.

[sukus21]

I have one of my own projects, not sure if that counts: https://github.com/sukus21/towizz

Not all of the included files have guards, as it only really became a problem for me later in development. Most files in include/entity have include guards though, and every once in a while, I have had to add new ones to existing include files.

[Rangi42]

Sorry, but after discussing this further, we've decided it's too fragile to support cross-platform after all. (A bit similar to C's #pragma once in that regard; almost every project I've seen uses plain old #ifndef include guards.)

[ISSOtm]

I haven't had a chance to explain the rationale more in depth, so please allow me to:

I think there is a major split between a file (as in, the object itself) and its contents.

Thus far, RGBASM has used the contents exclusively. Except that paths are used for INCLUDE and INCBIN, but merely as a way to name the contents of the file. (Likewise, we always pass paths through unaltered when printing.)

The major difference I see with INCLUDE_ONCE is that this one ascribes meaning to the path itself. But this presents several issues:

• The path and its contents are decorrelated. Two identical paths should point to the same contents (though e.g. FIFOs break that assumption), but two different paths may still point to identical contents. (Symlinks, or simply copies of the same file—I can list three different instances of hardware.inc within that project I'm currently working on.)
  Thus, attempting content deduplication based on paths, seems inherently fragile to me.
• Path resolution is far more platform-specific. While roughly everyone agrees on how to access file contents, the same is far from true for paths. I am not confident that this feature wouldn't introduce subtle platform-dependent breakage, and I would like to, as much as possible, avoid creating avenues that make it possible for RGBDS assembly to only compile on specific platforms. Because then, it ultimately puts more burden on the end users to remain aware of those avenues.

Ultimately, I recognise the feature's goals and usefulness, but I feel like they do not balance out the concerns above. This is what I have tried to express above, during the limited time I've been able to devote to RGBDS during my past weeks of holidays. (Please remember, this is not my job!)

I also understand that the implementation is simple, but I would like to respectfully point out that this simplicity may be hiding a lot of complexity—complexity that, if it doesn't deal with itself, is pushed onto the end user.

This is why I've asked for this feature request to be rejected. I hope it makes sense, and that you'll understand.