Skip to content

Rename files which are #included#708

Merged
NWilson merged 2 commits intomasterfrom
user/niwilson/file-renaming
Feb 27, 2025
Merged

Rename files which are #included#708
NWilson merged 2 commits intomasterfrom
user/niwilson/file-renaming

Conversation

@NWilson
Copy link
Member

@NWilson NWilson commented Feb 22, 2025

We have four files which have .c extensions, but which are actually #included rather than treated as their own compilation unit.

This goes against conventions - Autotools, CMake, and Bazel all assume that the .h/.c distinction indicates which files are compilation units.

We even have documentation telling users not to pass the bogus .c files to the compiler!

  • pcre2_jit_match.c.h
  • pcre2_jit_misc.c.h
  • pcre2_printint.c.h
  • pcre2_ucptables.c.h

@zherczeg
Copy link
Collaborator

zherczeg commented Feb 22, 2025

They have the .c extension because they are part of a .c file. Anyway, you can change them .h. However, I don't want a similar thing in the sljit compiler.

@PhilipHazel
Copy link
Collaborator

There are two kinds of confusion here. Seeing an inclusion for a .h file in a source makes me think it is some kind of header, not actual code, but I agree that seeing a .c file in a source directory suggests that it should be independently compiled. One way round this is to invent a new extension such as .inc but perhaps a clearer way would be to put included .c files into a subdirectory called "includedcode" (or whatever). Just my thoughts ... do what you think best.

@zherczeg
Copy link
Collaborator

Unfortunately I don't think there is a good solution here :( .c / .h represents a C source file, while .inc is something people (and tools) don't know. Another option is _c.h ending to show it is both, but looks strange. _inc.h is another option, although sounds confusing.

@NWilson
Copy link
Member Author

NWilson commented Feb 22, 2025

Oh dear, I'm sorry I opened a can of worms!

However, I don't want a similar thing in the sljit compiler.

Certainly not, I wouldn't touch that. None of the files in there need to be included by clients integrating PCRE2 into their build system.

I agree that seeing a .c file in a source directory suggests that it should be independently compiled.

More strongly than that, some tools actual mandate that behaviour, or make it a strong default that requires an override.

I admit I hadn't given it much thought.

Options for these "executable header" files:

  • .h. Downside: it's a header with side-effects, so confusing from that perspective
  • .c. I really would prefer not to keep this. We shouldn't go against the conventions assumed by tooling.
  • .inc wouldn't be treated as smoothly as .h; it's used by various file formats, include things like Makefile.inc for file fragments, so you wouldn't guarantee all clients are getting good tool defaults.
  • .c.h or _c.h or .inc.h or _inc.h... could do? The .foo.h options might be messy for Autoconf, which has special logic for trying to generate rules automatically based on file suffixes, eg foo.x.y from foo.x.

_impl.h is perhaps a best-of-both-worlds option? Sufficiently clear for people not to #include it by mistake, and "correct" with regards to tooling.

@zherczeg
Copy link
Collaborator

Renaming is always debatable because we should do it only once :) The inc is include, it does not add much to .h, which is also a header to be included. I like both _c.h and _impl.h both describe that the file contains code, and not a regular header. The _c emphasizes C code, although .h is only used by C, so I don't have a preference. The .x.y extensions are confusing for a human (for me at least).

@NWilson
Copy link
Member Author

NWilson commented Feb 23, 2025

OK, I like _impl.h a bit more than _c.h (as you point out, we already know it's C code).

Thanks for discussing!

@NWilson NWilson force-pushed the user/niwilson/file-renaming branch from adb6158 to b8c551c Compare February 26, 2025 22:27
@NWilson NWilson force-pushed the user/niwilson/file-renaming branch from 376bc80 to 7b011c2 Compare February 26, 2025 22:34
@NWilson
Copy link
Member Author

NWilson commented Feb 26, 2025

I made an update here. The files are renamed

pcre2_foo.cpcre2_foo_inc.h

This is after I realised that we already have pcre2_jit_simd_inc.h. I didn't want to rename those files as well, and I didn't want to have some files as _inc.h and some as _impl.h.

I don't really care too much about naming! I only felt that they shouldn't be .c files, given that build systems give special behaviour to .c files.

@zherczeg
Copy link
Collaborator

Anything is good for me as well. Probably we spent to much time on it.

@NWilson NWilson merged commit b79ee1d into master Feb 27, 2025
@NWilson NWilson deleted the user/niwilson/file-renaming branch February 27, 2025 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants