Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enables C preprocessing for .i files #16386

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

denizzzka
Copy link
Contributor

@denizzzka denizzzka commented Apr 15, 2024

Use case: Almost any C build system provides way to create *.i files during compilation (-save-temps flag). But usually C sources do not meet to the strict requirements of C11 standard needed by importC.

Ability to preprocess already preprocessed C files is useful in this case. We can just pass preprocessor over all *.i files recursively and it will convert files to a suitable for importC C version (replaces __inline__ to inline and so)

upd: This PR enables implicit preprocessing of .i files (same as works preprocessing .c files and included .h files)

This way we are spared from the need of enter/search/maintain/fetch of sets of definition flags for .c code used by D compiler - external C preprocessor does this job for us, just use generated .i files as bindings!

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @denizzzka! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#16386"

@dkorpel
Copy link
Contributor

dkorpel commented Apr 15, 2024

Can't you rename those back to .c? If .i gets preprocessed as well, what's the difference between .i and .c?

@denizzzka
Copy link
Contributor Author

denizzzka commented Apr 15, 2024

@dkorpel

Can't you rename those back to .c? If .i gets preprocessed as well, what's the difference between .i and .c?

.i can be preprocessed as non-standard C17 sources, for example:

__extension__ typedef long long _off64_t;
static __inline int _putchar_unlocked(int _c);

Such preprocessed file causes ImportC "Error: illegal combination of type specifiers" for both lines.
Why is this possible? Well, actually this is only possible with some other external compiler/preprocessor. But we already converting files using importc.h which also contains conversions for such non-standard lines.

Also, renaming to .c can't be used here, because .i files can contain such # lines, illegal in .c:

# 0 "/home/denizzz/Dev/Aeth/Espressif_IDE/esp-idf-v5.2.1/components/driver/gpio/gpio.c"
# 1 "/home/denizzz/Dev/БЦИ-1/mcu_software/build//"
# 0 "<built-in>"
# 0 "<command-line>"
# 1 "/home/denizzz/Dev/Aeth/Espressif_IDE/esp-idf-v5.2.1/components/driver/gpio/gpio.c"

# 41 "/home/denizzz/.espressif/tools/riscv32-esp-elf/esp-13.2.0_20230928/riscv32-esp-elf/riscv32-esp-elf/sys-include/machine/_default_types.h" 3 4
typedef signed char __int8_t;

In fact, for now this patch just applies preprocessor (with onboard importc.h) for each *.i file as for .c files

I think there is no need to differentiate between .i and .h/.c files - all of them need to be preprocessed by a preprocessor with same importc.h config

@denizzzka
Copy link
Contributor Author

denizzzka commented Apr 15, 2024

I think there is no need to differentiate between .i and .h files - all of them need to be preprocessed by preprocessor with same importc.h config

Perhaps here is also needed compiler flag like: “Strict compliance with the C11 standard” which disables .i preprocessing and importc.h for .c/.h

@ibuclaw
Copy link
Member

ibuclaw commented Apr 16, 2024

@dkorpel

Can't you rename those back to .c? If .i gets preprocessed as well, what's the difference between .i and .c?

.i can be preprocessed as non-standard C17 sources, for example:

__extension__ typedef long long _off64_t;
static __inline int _putchar_unlocked(int _c);

Such preprocessed file causes ImportC "Error: illegal combination of type specifiers" for both lines.

In other words, the file is preprocessed, but gcc/clang support parsing all the alternative names for C keywords (before they got standardised).

@denizzzka
Copy link
Contributor Author

denizzzka commented Apr 16, 2024

Such preprocessed file causes ImportC "Error: illegal combination of type specifiers" for both lines.

In other words, the file is preprocessed, but gcc/clang support parsing all the alternative names for C keywords (before they got standardised).

@ibuclaw yes

I do not like name -strictC11 for a flag because maybe we will support C17 in the future.
Suggestions? -strictC?

Another question: should this option be hidden from --help screen if platform isn't supports preprocessor?

@denizzzka
Copy link
Contributor Author

Looks like MS preprocessor can't preprocess already preprocessed files :-(

@WalterBright
Copy link
Member

ImportC runs the preprocessor when compiling .c files. This relies on the file druntime/src/importc.h being #include'd, which dmd does by passing a switch to #include it on the command to the C preprocessor program, like /FIimportc.h for MSVC. importc.h and -HIimportc.h for Posix, has a lot of macros in it which adapt the host C compiler's non-standard quirks to something ImportC can handle, like those extra keywords.

Therefore, if one is using a C build system to cache generated .i files, add the /FI or -HI switch to that build script, and it should then work fine with ImportC.

@denizzzka
Copy link
Contributor Author

denizzzka commented Apr 16, 2024

@WalterBright

ImportC runs the preprocessor when compiling .c files.

More precisely, before compilation. And .h files #included in .c files are preprocessed too. This is important, see below

This relies on the file druntime/src/importc.h being #include'd, which dmd does by passing a switch to #include it on the command to the C preprocessor program, like /FIimportc.h for MSVC. importc.h and -HIimportc.h for Posix, has a lot of macros in it which adapt the host C compiler's non-standard quirks to something ImportC can handle, like those extra keywords.

Therefore, if one is using a C build system to cache generated .i files, add the /FI or -HI switch to that build script, and it should then work fine with ImportC.

As I understand you, my PR is about another thing: it eliminates the unfair difference between importing "non-standard" .c (which automatically preprocessed with mentioned above importc.h) and importing .i (which must be strictly comply to the C11 standard). And here is no way to obtain such .i file except manually calling C preprocessor with -include importc.h (but then why preprocessor is automatically called for .c by the D compiler?)

@WalterBright
Copy link
Member

And here is no way to obtain such .i file except manually calling C preprocessor with -include importc.h

Doesn't every C build system have a means to add switches to the preprocessor?

@denizzzka
Copy link
Contributor Author

denizzzka commented Apr 16, 2024

And here is no way to obtain such .i file except manually calling C preprocessor with -include importc.h

Doesn't every C build system have a means to add switches to the preprocessor?

This is what I meant by "manually"

But we don’t preprocess regular .c files in this manner - preprocessor for us called by D compiler. I think for .i files should be same behaviour

@denizzzka
Copy link
Contributor Author

denizzzka commented Apr 16, 2024

Also, in case of separate run of preprocessor we should know where is importc.h, but path to it isn't standartized and maybe for different compilers it will be different

@denizzzka denizzzka marked this pull request as ready for review April 17, 2024 10:23
@denizzzka
Copy link
Contributor Author

New option removed, replaced by --cpp= (with empty value)

@denizzzka denizzzka requested a review from ibuclaw as a code owner April 17, 2024 12:04
@denizzzka denizzzka force-pushed the preprocess_i_files_too branch 5 times, most recently from 1548cd2 to a89e96f Compare April 19, 2024 11:53
@WalterBright
Copy link
Member

As I understand you, my PR is about another thing: it eliminates the unfair difference between importing "non-standard" .c (which automatically preprocessed with mentioned above importc.h) and importing .i (which must be strictly comply to the C11 standard). And here is no way to obtain such .i file except manually calling C preprocessor with -include importc.h (but then why preprocessor is automatically called for .c by the D compiler?)

It has nothing to do with fairness. I don't understand why the user cannot add a switch to their build to add in the necessary .h file.

@denizzzka
Copy link
Contributor Author

denizzzka commented Apr 23, 2024

It has nothing to do with fairness.

(I meant what .c files is 1-st class passengers, but .i isn't for some unknown reason)

.c files can be processed by an external preprocessor too, but D compiler invokes preprocessor by itself. But not for .i files.

I don't understand why the user cannot add a switch to their build to add in the necessary .h file.

It's just pointless extra work that could very easily be avoided. (For hypotetical rare cases when importc.h incompartible with used C code this PR allows to disable preprocessor at all)

@WalterBright
Copy link
Member

but .i isn't for some unknown reason

The reason is .i is for preprocessed files, so no preprocessor should be run on them.

@denizzzka
Copy link
Contributor Author

denizzzka commented Apr 29, 2024

The reason is .i is for preprocessed files, so no preprocessor should be run on them.

Why not? By default no one knows (from outside of the D compiler, I mean) that actually .i file implictly preprocessed again. Compiler pretends what it just understands all theese C dialects. After all, that was the idea of calling preprocessor with importc.h internally?

@dkorpel
Copy link
Contributor

dkorpel commented Apr 29, 2024

Also, renaming to .c can't be used here, because .i files can contain such # lines, illegal in .c:

I tried, and dmd compiles that snippet just fine.

@dkorpel
Copy link
Contributor

dkorpel commented Apr 29, 2024

Oh, it's only a problem on Windows

version (Windows) enum canPreprocessTwice = false;

@denizzzka
Copy link
Contributor Author

@dkorpel yes, and I left this line instead of version (Windows) because the next logical step would be detection and support Clang on Windows (Clang isn't need trick with temporary file)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants