Closed sukus21 closed 6 days ago
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?
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
).
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
include_once
is a wonderful idea, that is so much better!
Out of two popular include
d "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 -P
reinclude 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.
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.
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.)
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.
...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!
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!
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".)
Also if pokecrystal ends up doing something like https://github.com/pret/pokecrystal/pull/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.
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.
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.)
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:
hardware.inc
within that project I'm currently working on.)
Thus, attempting content deduplication based on paths, seems inherently fragile to me.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.
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 aspragma 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 likeINCGUARD
?).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 :)