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

Add C source files to the list of sourceFiles as well #2521

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

Conversation

cschlote
Copy link
Contributor

Starting with the next DMD version 2.101 the compiler also processes C files. Added some code to pickup the C files in the source directories as well.

Tested the code with some example project for ImportC features: https://gitlab.vahanus.net:dlang/examples/importc-tests.git

The PR might need additional checks for available DMD/LDC/GDC versions, so that C files are only added, if the compiler
can process it. Or at least output some warnings/hints. Please advise.

Starting with the next DMD version 2.101 the compiler also processes
C files. Add some code to pickup the C files in the source directories
as well.
@rikkimax
Copy link
Contributor

See my comment here: #2270

Note how the changes there do not match this PR.

This needs a lot more thinking about, than just adding some filters.

@cschlote
Copy link
Contributor Author

At the moment you must either pass an explicite list of C sourcefiles in dub.json (see forum posts), or use some 'preBuild' command to compile your C code to object files and add them to some other explicite list for the linker.

What we need in dub is that C files are just picked up like any other D file as a valid source for the compiler. The C header files are intentionally left out here, because it is bad style to have real functions in a C header. Yes, I know, people are doing such things. It is one of the bad habbits of C. If you want to include a C header, just wrap it into a C file, which is then compiled by the D compiler. See the mentioned example project for wrapp ZSTD header.

I strongly recommend to add working support for ImportC ASAP.

The cleanups you mentioned can be done later after some working solution is provided now, not next year. We are talking about really small changes here. Also the other PR is pretty small. There is no danger to mess up sources with some complicated intermediate solution here.

But with the next DMD release we should be able to use ImportC directly and easily instead of explicite sourceFile lists, cumbersome workarounds and preBuild/Generate hacks.

Straight and easy support for using C files in DUB is a must with the next DMD release. It will help a lot when we want to promote D as a suitable language alternative for C/C++ programmers.

@rikkimax
Copy link
Contributor

Straight and easy support for using C files in DUB is a must with the next DMD release. It will help a lot when we want to promote D as a suitable language alternative for C/C++ programmers.

It won't "just work", it simply isn't ready for most people to use.

We don't even distribute a macro preprocessor!

This needs proper thinking about and implementation. Dub simply isn't a c build or package manager, it's a massive scope increase.

@rikkimax
Copy link
Contributor

I'm currently on BeerConf, if you'd like to join and talk about the scope I'd be keen to help with fleshing it out.

Splitting dub roles up between package/build managers is something I am quite interested in as it is a known limitation that needs resolving.

@ryuukk
Copy link
Contributor

ryuukk commented Oct 30, 2022

I just read your comment on the linked PR, and you make a valid point, +1 for merge

Endless discussion about it will get us nowhere, i opened my PR 5 months ago, and since then nothing was done

So let's merge this

@cschlote
Copy link
Contributor Author

I just digging through the Dub sources to get some overview.

There are many places, where just a '.d' extension is checked, with inline code like .endsWith(".d") or similar. We need to change that to something like isSourceFile(), isHeaderFile() and similar. Then all the checks would be at one place.

About the Scope change: The D compilers already accepts .c, .h and .i files as additional inputs, beside .d and .di files.

  • Source files are .d, .c and .i files => isSourceFile(f)
  • Header files are .di and .h files => isHeaderFile(f)
  • The C preprocessor needs to be called with appropriate -I (include) args. But this should be handled by the D compiler, because the compiler is calling the prepro. And Dub is passing import and additional source directories to the compiler.

So the scope change can't be that much. We only collect some more files, which can now be passed to the D compiler as valid inputs. So the simple approach would be to replace all .endsWith(".d") et al. stuff with proper functions. And also add additional collectFiles() calls at the right places.

This should already cover many usecases. If combined with a flag/switch in the JSON/SDL file, we can elaborate the code without disturbing existing stuff.

And of cause we can rework code for package/build managers as well. But this we should discuss somewhere online. I need some more understanding about current package/build managers issues and the proposed solutions.

I'm really eager to support these changes. It really simplify a lot of my D projects at work. Removing unnessesary pain when interfacing with C libs and code is for sure something, which helps to promote D.

@rikkimax
Copy link
Contributor

Okay, now we're getting some analysis :)

There will also need to be functions like haveImportCSupport, isImportCFile.

Even if the compiler supports it (we have no idea on the ETA for gdc/ldc is @ibuclaw @kinke) it may still not have everything it needs to work like the preprocessor or header files @WalterBright (I don't see any preprocessor in beta).

@cschlote
Copy link
Contributor Author

Added a new draft PR #2522 for the ImportC topic. There we can continue the discussion and work out the required changes.

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.

None yet

3 participants