Skip to content

[WIP] Generate C++ header files from public extern (C++) declarations#8591

Closed
ibuclaw wants to merge 7 commits intodlang:masterfrom
ibuclaw:dtoh
Closed

[WIP] Generate C++ header files from public extern (C++) declarations#8591
ibuclaw wants to merge 7 commits intodlang:masterfrom
ibuclaw:dtoh

Conversation

@ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Aug 21, 2018

Takes #5082 and converts it into a standalone program.

Still needs a bit of work, documentation, command-line option handling maybe to turn on/off certain parts.

Some bugs that I reported are still present, such as #5082 (comment)

Using the auto-generated header in the C++ unittest works at least.

@ibuclaw ibuclaw added the Review:WIP Work In Progress - not ready for review or pulling label Aug 21, 2018
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

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 fetch digger
dub run digger -- build "master + dmd#8591"

@jacob-carlborg
Copy link
Contributor

I would recommend using the Dub package. Also building the new tool as a Dub package.

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it would save a lot of code in the main method if this would use the DMD dub package.

src/dmd/dtoh.d Outdated
"safe.d", "sapply.d", "semantic2.d", "semantic3.d", "sideeffect.d", "statement.d",
"statementsem.d", "staticassert.d", "staticcond.d",
"target.d", "templateparamsem.d", "tokens.d", "traits.d", "typesem.d", "typinf.d",
"utf.d", "utils.d",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use globbing here like we do for the dub package or build.d, s.t. we have less files to update whenever a file gets moved?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, this part was carried over from the old PR.

For sure am open to design improvements - the most important part is the blacklisting of certain modules that we retain manual updates for.

// Generate special static inline function.
if (cd.isIdentifierClass())
{
buf.writestring(" static inline Identifier *idPool(const char *s) { return idPool(s, strlen(s)); }\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this special casing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this doesn't exist in D for obvious reasons.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I was more asking: shouldn't we do this in a more general-purpose way.

src/dmd/dtoh.d Outdated
global.params.isLP64 = global.params.is64bit;

global.path = new Strings();
global.path.push(__FILE_FULL_PATH__.dirName.buildPath("../../../druntime/src/").toStringz());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fragile if druntime is checkout out in a different directory (I think the LDC people do this).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes our path is $LDCROOT/runtime/druntime.

@wilzbach
Copy link
Contributor

SemaphoreCI failure is due to the dub package trying to include dtoh:

../../src/dmd/dtoh.d(103,23): Deprecation: Using super as a type is deprecated. Use typeof(super) instead
../../src/dmd/dtoh.d(131,23): Error: function `dmd.dtoh.genCppFiles.ToCppBuffer!(ASTCodegen).ToCppBuffer.visit` conflicts with alias dmd.dtoh.genCppFiles.ToCppBuffer!(ASTCodegen).ToCppBuffer.visit at ../../src/dmd/dtoh.d(103,9)
../../src/dmd/dtoh.d(1140,19): Error: template instance `dmd.dtoh.genCppFiles.ToCppBuffer!(ASTCodegen)` error instantiating

@jacob-carlborg
Copy link
Contributor

Related [1]. Perhaps join forces?

[1] https://forum.dlang.org/thread/oikrrejcengbjivsyzws@forum.dlang.org

@jacob-carlborg
Copy link
Contributor

@TurkeyMan this should be interesting for you.

@ibuclaw
Copy link
Member Author

ibuclaw commented Aug 22, 2018

@jacob-carlborg this differs in that it is not intended to be a general purpose tool. Same as the tool used to convert dmd to D. This handles only dmd specific style and quirks.

@kinke
Copy link
Contributor

kinke commented Aug 23, 2018

Nice job Iain, thanks!

@PetarKirov
Copy link
Member

It would be better to make the hard-coded lists and paths command-line / or environment parameters.

@TurkeyMan
Copy link
Contributor

What's going on with this? I could really use this like, yesterday.

@ibuclaw
Copy link
Member Author

ibuclaw commented Sep 29, 2018

@TurkeyMan - it's hardly meant for general purpose, there are many special cases that look for dmd-specific code.

If it gets used at all, it'll be part of the build process for generating headers that gdc and ldc can include. No need to check what is and isn't in sync.

@TurkeyMan
Copy link
Contributor

Right... I want to do an experiment where I define some struct's and functions in D, but they are extern(C++), and the .h file needs to be emit by the D compiler. Is that the goal here? It sounds like the same goal; keeping the C++ headers in sync.
This would be a massive enabler for my current project.

@thewilsonator
Copy link
Contributor

@TurkeyMan I'll do this up as a dub package once #8776 goes in.

@WalterBright
Copy link
Member

Is this still a WIP?

@thewilsonator
Copy link
Contributor

This needs:

  • the druntime path to be fixed for LDC
  • made to work for non-linux, and 32-bit

It would also be good to have the DMD-specific stuff factored out.

@ibuclaw
Copy link
Member Author

ibuclaw commented Oct 26, 2018

Is this still a WIP?

Yes, as I have done no testing whatsoever against gdc and auto-generated headers.

I've been too busy with work on other things to look back at this (you should know what other things are :-)

The cxxfrontend tests that are in dmd only cover the surface of the C++ bindings. That the generated header is compilable and the running program passes all tests is promising though.

As I've said before, this not meant to be general purpose, I would suggest maybe dpp is better suited for you than this utility tool.

@thewilsonator
Copy link
Contributor

As a general purpose dub-package-to-be: https://github.com/thewilsonator/dtoh

N.B. also completely untested.

@jacob-carlborg
Copy link
Contributor

I would suggest maybe dpp is better suited for you than this utility tool.

dpp is for the other way.

@TurkeyMan
Copy link
Contributor

TurkeyMan commented Oct 26, 2018

As I've said before, this not meant to be general purpose, I would suggest maybe dpp is better suited for you than this utility tool.

dpp is the wrong way around; we want to emit .h files with the extern(C++) symbols like .di files work today... in this case, the D code should be the authoritative code. I don't want C++ injecting noise into this particular codebase.

Is there a path to making this work general purpose in the way I say?
extern(C++) doesn't support anything that C++ doesn't support; the mapping should be cleanly possible.

thewilsonator added a commit to thewilsonator/dmd that referenced this pull request Oct 28, 2018
For DMDFE as a library so that tools can populate `global.params` correctly.

Required for https://github.com/thewilsonator/dtoh the dub package of dlang#8591
@thewilsonator thewilsonator force-pushed the dtoh branch 2 times, most recently from 1d2f88b to 9b9e45a Compare November 18, 2018 06:16
}
// Read the configurarion file
auto inifile = File(global.inifilename);
inifile.read();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thewilsonator
Copy link
Contributor

There is #9971 now, can this be closed?

@ibuclaw
Copy link
Member Author

ibuclaw commented Jun 10, 2019

If they're willing to take the baton in this relay race.

@wilzbach wilzbach closed this Jun 10, 2019
@wilzbach
Copy link
Contributor

It's Edi's GSoC project, so he'll be working full-time on this over the next weeks.

@ibuclaw ibuclaw added the Feature:dtoh C++ header generation label Sep 28, 2020
@ibuclaw ibuclaw deleted the dtoh branch December 29, 2020 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:dtoh C++ header generation Review:WIP Work In Progress - not ready for review or pulling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants