Expose D frontend via DUB#7425
Conversation
|
Thanks for your pull request, @wilzbach! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
Btw @WalterBright this happens automatically because of Line 34 in f27ee71 If you mind about this, we should improve the mappings. |
dub.sdl
Outdated
|
|
||
| sourceFiles \ | ||
| "src/ddmd/astbase.d" \ | ||
| "src/ddmd/astbasevisitor.d" \ |
There was a problem hiding this comment.
Same change required as inhttps://github.com//pull/7415 (atm moment the DUB file is outdated).
dub.sdl
Outdated
| "src/ddmd/libmach.d" platform="osx" | ||
|
|
||
| dependency "dmd:parser" version="*" | ||
| dependency "dmd:lexer" version="*" |
There was a problem hiding this comment.
FWIW splitting up dmd in four packages creates a noticeable overhead in terms of DUB compile times (compared to one big library file).
There was a problem hiding this comment.
I thought it would be nice to be able to only pull in the lexer, i.e. for source code highlighting. But since Dub will currently download the whole DMD package anyway it doesn't matter that much.
Although, it would be nice to have some kind of enforcement of the separation between the different phases, to avoid that the lexer suddenly starts to depend on the rest of the front end.
There was a problem hiding this comment.
Well I hope that in practice one doesn't need to constantly recompile libdmd.a, so it shouldn't matter too much.
| extern (C) void* allocmemory(size_t m_size) nothrow | ||
| { | ||
| return GC.malloc(m_size); | ||
| } |
There was a problem hiding this comment.
It seems like version(GC) isn't used actively (at least by anything that is part of the testsuite/CIs).
test/dub_package/source/app.d
Outdated
| Expression._init(); | ||
| Objc._init(); | ||
| builtin_init(); | ||
| } |
There was a problem hiding this comment.
Is there any chance we can move a generic initialization function into the DMD codebase?
There was a problem hiding this comment.
Currently this is handled in mars.d, which contains the main function of DMD. This file should not be included in the Dub package, but I don't see why we can't move this to a separate function, called initFrontEnd which can be called both from the compiler and the Dub package. Perhaps we can add a new file with a shared constructor that is only part of the Dub package that calls this initFrontEnd function.
There was a problem hiding this comment.
Perhaps we can add a new file with a shared constructor that is only part of the Dub package that calls this initFrontEnd function.
Yeah, I think this would be the best way to move forward. This file could also contain other convenience, modern D wrapper. How about ddmd/libdmd.d, ddmd/library.d, or ddmd/frontend.d?
There was a problem hiding this comment.
I don't like ddmd/libdmd.d, the others are fine, I think I prefer ddmd/frontend.d. But I don't want to limit ourselves to this file. I think we should look at what we want to add and add it to the modules where it makes most sense. I don't mind adding several new modules and/or packages if that makes more sense. I don't have any problems with having only one symbol in a module if it doesn't fit anywhere else.
test/dub_package/source/app.d
Outdated
| import ddmd.id : Id; | ||
|
|
||
| init; | ||
| addImports(["/usr/include/dlang/dmd/"]); |
There was a problem hiding this comment.
This is the worst bit about this PR. While, for this test we could mock object.d, I think we should show potential users how they can add import paths as they most likely want to do so.
Ideas:
- mock
object.d - use sth. like
../../../druntime/import(i.e. depend on existing repo layout) - call DMD and search config strings
- refactor config parsing from
mars.d, s.t. it can be used here
test/dub_package/source/app.d
Outdated
| import ddmd.tokens : TOKeof; | ||
| import ddmd.id : Id; | ||
|
|
||
| init; |
There was a problem hiding this comment.
We still need to define all predefined versions, but addDefaultVersionIdentifiers is private.
Imho if I would be about to write an IDE plugin, I wouldn't want to keep this list in sync if DMD already has the logic.
For example, to import Phobos on Linux, I need this list:
import ddmd.cond : VersionCondition;
VersionCondition.addPredefinedGlobalIdent("DigitalMars");
VersionCondition.addPredefinedGlobalIdent("Posix");
VersionCondition.addPredefinedGlobalIdent("linux");
VersionCondition.addPredefinedGlobalIdent("ELFv1");
VersionCondition.addPredefinedGlobalIdent("LittleEndian");
VersionCondition.addPredefinedGlobalIdent("CRuntime_Glibc");
VersionCondition.addPredefinedGlobalIdent("D_Version2");
VersionCondition.addPredefinedGlobalIdent("all");
VersionCondition.addPredefinedGlobalIdent("D_InlineAsm_X86_64");
VersionCondition.addPredefinedGlobalIdent("X86_64");
VersionCondition.addPredefinedGlobalIdent("D_HardFloat");
So any chance we can refactor addDefaultVersionIdentifiers and expose it?
test/dub_package/source/app.d
Outdated
| res.append(a); | ||
| } | ||
| global.path = res; | ||
| } |
There was a problem hiding this comment.
Well, obviously it would be nice if the dmd "library` contains a couple of high-level wrapper, s.t. a potential author only has to interact with its shiny, modern D APIs.
e476081 to
5a7e4e7
Compare
a908012 to
78a97fb
Compare
|
OK, so I added a rudimentary -> this should now work on Travis
-> I created Other points (now not displayed anymore):
|
197f8a9 to
c054b31
Compare
| @@ -1,6 +1,13 @@ | |||
| # Ignore all binary files (files without an extension) | |||
There was a problem hiding this comment.
As hopefully more examples will be added, ignoring all binary files in this directory avoids frequent updates of this file.
There was a problem hiding this comment.
I think a cleaner solution would be put all dub artifacts in generated/ as well, using targetPath (see http://code.dlang.org/package-format?lang=sdl).
c054b31 to
211ec51
Compare
dub.sdl
Outdated
| "src/ddmd/templateparamsem.d" \ | ||
| "src/ddmd/semantic.d" \ | ||
| "src/ddmd/sideeffect.d" \ | ||
| "src/ddmd/statement.d" \ |
There was a problem hiding this comment.
The indentation is wrong here.
dub.sdl
Outdated
| "src/ddmd/typinf.d" \ | ||
| "src/ddmd/utils.d" \ | ||
| "src/ddmd/statement_rewrite_walker.d" \ | ||
| "src/ddmd/statementsem.d" \ |
There was a problem hiding this comment.
The indentation is wrong here.
test/dub_package/README.md
Outdated
|
|
||
| If you want to see the log output or want to pass additional options, use `--single`: | ||
|
|
||
| dub --single -v lexer.d |
There was a problem hiding this comment.
This line and line 10 doesn't have the same indentation. For code blocks I recommend the triple backticks.
| @@ -0,0 +1,17 @@ | |||
| DMD as a library | |||
| ================ | |||
There was a problem hiding this comment.
Personally I prefer the hash # for headers. Then I only need one character and don't have to add/remove characters if the title changes.
There was a problem hiding this comment.
Yeah I usually use a mix of these styles in my markdown: === for the title and # for all headers within the file as the title usually changes seldom, but has more emphasis this way.
Anyhow I wouldn't mind at all to use a # if that's important, but I think there are more pressing issues than which flavor of Markdown should be used?
There was a problem hiding this comment.
No, it's not that important. That's why I said it's my personal preference.
test/dub_package/frontend.d
Outdated
| initDMD; | ||
| findImportPaths.addImports; | ||
|
|
||
| auto parse(Module m, string code) { |
There was a problem hiding this comment.
Opening curly braces on their own lines.
211ec51 to
c41a200
Compare
|
(addressed the comments & hopefully found the issue for the failing Travis test.) |
dub.sdl
Outdated
| "src/ddmd/safe.d" \ | ||
| "src/ddmd/blockexit.d" \ | ||
| "src/ddmd/printast.d" \ | ||
| "src/ddmd/gluelayer.d" |
There was a problem hiding this comment.
Minor detail, it would be nice if the list was sorted alphabetically, unless it's sorted in some other way that I fail to see now.
dub.sdl
Outdated
| "src/ddmd/libelf.d" platform="linux" | ||
|
|
||
| sourceFiles "src/ddmd/scanmach.d" \ | ||
| "src/ddmd/libmach.d" platform="osx" |
There was a problem hiding this comment.
The last three files, I know they're not technically not part of the backend but I think they feel more geared towards the backend or code generation. Do we want to include them?
There was a problem hiding this comment.
Currently they are required by some bits of the frontend (see below). Though I'm pretty sure, that these parts could versioned out in the future.
Remove libelf.d
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(lib.o):(.data.rel.ro+0x28): undefined reference to `_D4ddmd6libelf12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(lib_532_2d5.o): In function `_D4ddmd3lib7Library7factoryFZCQBcQBaQz':
/home/seb/dlang/dmd/src/ddmd/lib.d:56: undefined reference to `LibElf_factory()'
collect2: error: ld returned 1 exit status
Error: linker exited with status 1
Remove gluelayer.d
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(dclass.o):(.data.rel.ro+0x20): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(denum.o):(.data.rel.ro+0x10): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(dmodule.o):(.data.rel.ro+0x28): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(dsymbol.o):(.data.rel.ro+0x30): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(dsymbolsem.o):(.data.rel.ro+0xd8): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(dsymbolsem_39a_657.o): In function `Semantic3Visitor::visit(FuncDeclaration*)':
/home/seb/dlang/dmd/src/ddmd/dsymbolsem.d:1043: undefined reference to `retStyle(TypeFunction*)'
/home/seb/dlang/dmd/src/ddmd/dsymbolsem.d:1097: undefined reference to `retStyle(TypeFunction*)'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(expression.o):(.data.rel.ro+0x40): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(func.o):(.data.rel.ro+0x28): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(mars.o):(.data.rel.ro+0x20): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(mtype.o):(.data.rel.ro+0x38): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(objc.o):(.data.rel.ro+0x60): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(objc_5ba_3c6.o): In function `_D4ddmd4objc9Supported6__ctorMFZCQBfQBdQBb':
/home/seb/dlang/dmd/src/ddmd/objc.d:185: undefined reference to `objc_initSymbols()'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(statement.o):(.data.rel.ro+0x30): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(statementsem.o):(.data.rel.ro+0xd0): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(statementsem_632_9d9.o): In function `StatementSemanticVisitor::visit(AsmStatement*)':
/home/seb/dlang/dmd/src/ddmd/statementsem.d:3923: undefined reference to `asmSemantic(AsmStatement*, Scope*)'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(typinf.o):(.data.rel.ro+0x48): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(aggregate.o):(.data.rel.ro+0x18): undefined reference to `_D4ddmd9gluelayer12__ModuleInfoZ'
/tmp/.dub/build/dmd-~dub-frontend/library-debug-linux.posix-x86_64-dmd_2077-A550EF4C39A32552F8FE470A957BC323/libdmd_frontend.a(typinf_680_468.o): In function `genTypeInfo(Type*, Scope*)':
/home/seb/dlang/dmd/src/ddmd/typinf.d:67: undefined reference to `toObjFile(Dsymbol*, bool)'
There was a problem hiding this comment.
I see. I think we can do some refactoring later to remove these dependencies if we want to.
BTW, I think lib.d is for generating libraries, which I don't think we need.
src/ddmd/frontend.d
Outdated
| */ | ||
| void initDMD() | ||
| { | ||
| import ddmd.globals : global; |
There was a problem hiding this comment.
This is not used as far as I can see.
test/dub_package/frontend.d
Outdated
| import ddmd.globals : global; | ||
|
|
||
| global._init; | ||
| global.params.isLinux = 1; |
There was a problem hiding this comment.
Do we need to worry about this when the tests are also run on macOS?
c41a200 to
682e3f7
Compare
|
LGTM 👌. |
682e3f7 to
0c91dab
Compare
dub.sdl
Outdated
| versions "NoMain" | ||
| sourcePaths | ||
|
|
||
| sourceFiles \ |
There was a problem hiding this comment.
This list will need to be updated every time a source file is added or removed. Is there a simple way to automate it?
There was a problem hiding this comment.
One way is to move every file that is part of the Dub package to its own directory, like frontend, then the whole directory can be included with sourcePaths.
There was a problem hiding this comment.
DUB also supports excludedSourceFiles for removing files from a sourcePaths set
There was a problem hiding this comment.
Give it a try. Seems to be simple enough, though it would have been easier with a better layout of DMD (see the exclude file globs) and I don't know how fast the globbing is implemented in DUB (though it's a handful of files)...
If you want to cleanup the house, some ideas:
- move everything related to the IR ->
dmd.ir - move everything related to reading & writing object files
src/dmd/{scan,lib}{elfmach,mscoff,omf}.d->dmd.obj - group other files into folders too, e.g. the
dmd.parserordmd.visitor
0c91dab to
6c18da4
Compare
1a86af1 to
ffb1b87
Compare
0a0ce99 to
3799247
Compare
6b4ad57 to
94c6525
Compare
94c6525 to
711f365
Compare
So I finally got Travis to pass :) Finally the "simple" frontend example now has more than 150 lines, so in a follow-up PR we should try to factor out common logic to |
|
@andralex @jacob-carlborg @RazvanN7 (or other) okay to move forward with this as an initial version? I would like to look into @joakim-noah's report that we can't due a full semantic on Phobos yet and also start to build stuff on top of this ;-) |
|
Yes, please move forward 👌. |
|
Auto-merge toggled on |
RazvanN7
left a comment
There was a problem hiding this comment.
Other than some minor comments it looks good. The next step should be to expose more semantic methods in a way that is easy to use, for example: the one which I find very important is the search method which does symbol resolution.
| import std.stdio; | ||
|
|
||
| // add import paths | ||
| void addImports(T)(T path) |
There was a problem hiding this comment.
Why isn't this in frontend.d? (src/dmd/frontend.d)
| } | ||
|
|
||
| // finds a dmd.conf and parses it for import paths | ||
| auto findImportPaths() |
There was a problem hiding this comment.
Why isn't this in frontend.d ?
| } | ||
| }); | ||
|
|
||
| void semantic() |
There was a problem hiding this comment.
This should certainly be in frontend.d and we could call it fullSemanticAnalysis or something along these terms.
There was a problem hiding this comment.
Unless there's a reason to not run the full semantic analysis I don't think the full prefix is necessary. I hope that we can keep the three phases an implementation detail.
There was a problem hiding this comment.
I'm not sure this is possible. As a tool developer you may want to visit the AST while it is in a certain state. Given the fact that the semantic methods modify the AST quite a lot you might want to take a look at it before a certain semantic pass is done. For example: enums are evaluated at compile time (during semantic3) ; in order to do those modifications the semantic3 stage alters the AST; as a tool developer I may want to do something with the original AST which is not longer possible given that semantic3 has modified it.
There was a problem hiding this comment.
Hehe that's exactly why I didn't want to push this into frontend.d for now ;-)
There was a problem hiding this comment.
That's not a problem. You have the individual semantic passes, but if you don't care about that you can just call FullSemanticAnalysys or some other name that you find suited.
There was a problem hiding this comment.
Please let leave it as is and continue improve it after this is merged.
Yes they should be (I think I mentioned this a couple of times), but for now I didn't want to push anything to |
|
@wilzbach Sure thing. I was just suggesting the next step after this PR is merged. |
Yes, let's continue the discussion at #7485 |
Allows to use the entire frontend as a DUB package.
There's still an ugly lot of initialization needed and two hard-coded bits (platform, importPaths), but one can run through all semantic phases which is more than needed if we want to take the DMD as a library project serious ;-)
I will comment on the individual lines for which I would like to find a better solution.
Also note that I didn't add this to the test scripts yet as I am waiting for #7415 (allows to test the DUB package on Travis).