Implement -rb switch for recursive build and -rx switch for exclusion lists.

[ghost]

http://d.puremagic.com/issues/show_bug.cgi?id=9896

[ghost]

Preferably this would be part of the ini file, however that's not part of the repository so I can't change it.

[CyberShadow]

It is now!

[ghost]

Ok great. But to avoid build failures the process would have to be:

• Merge pull (after review etc.)
• Add default exclusions to ini files
• Make another pull removing hardcoding

[CyberShadow]

Build failures? In the autotester? The autotester doesn't use sc.ini files from the repository?

Because it doesn't affect users (I assume there is no change in behavior if -rb is not specified).

[ghost]

I meant if I update sc.ini right now and add -rb to the flags it will fail since this pull isn't merged yet.

[CyberShadow]

Sorry, what do you mean? Why not add the sc.ini changes in this pull? The ini files are stored in this repository, not in e.g. the installer repo, so since the ini changes will get pulled only together with the DMD changes, why would it cause build failures?

[CyberShadow]

Also, I don't think -rb should be in the .ini file, at least not for now. Just the -rx defaults.

[ghost]

Ah I didn't know they're here, I'll take a look.

[CyberShadow]

I think the top-level makefile already sets the OBJ variable using a similar check.

[CyberShadow]

(Reposting) The top-level makefile declares the DSEP and OBJ variables.

[ghost]

Thanks I'll fix it up.

[MartinNowak]

Please spend the time to make a DIP and reach a solid consensus for this feature and how it will integrate with rdmd.

• We're using module->importedFrom in several places to attach template symbols and typeinfo
  to an object file. If you're actually building the module then importedFrom will lose that semantic
  and should be either set to the module itself or we need to fix all occurrences and use a better
  mechanism.
• I'm wondering if it could happen that you run semantic3 on a module that didn't had its
  previous passes already run. But there is an assertion for this and ImportStatement seems to handle it
  correctly.
• You could simply drop the glob and allow to specify modules or packages.
  -rbfoo => everything under pkg foo or only foo if foo is a module
  -rb => everything
• How about support for whitelists as well as blacklists, -rbfoo -rxfoo.bar.
• Please factor out the match function.

It's quite confusing but maybe still helpful to look at https://github.com/dawgfoto/dmd/tree/DIP11 which supported a pragma(build, <mod-or-pkg>).

[CyberShadow]

Hi Martin,

This patch is a follow-up to a discussion in the newsgroup. The discussion is linked to in the corresponding Bugzilla issue.

I'm not sure how a DIP would be appropriate, considering that the issue can be summarized in one sentence (move rdmd's recursive compilation feature into the compiler). The actual flags are currently not as important, considering they are mainly used as an internal interface with rdmd. Changing dmd's interface (command-line) to be more user-friendly to take advantage of this new feature can be a separate enhancement, worthy of a separate discussion.

You could simply drop the glob and allow to specify modules or packages.

This proposal was also discussed in the newsgroup.

How about support for whitelists as well as blacklists, -rbfoo -rxfoo.bar.

This is equivalent to -rb foo -rxfoo.bar. Do you mean nested exclusions (whitelist a package that would have been covered by a blacklist)? Can you present a use case for such a feature?

I can't speak of matters related to compiler internals.

[CyberShadow]

(Nitpick) No . before ${OBJ} (OBJ already starts with .)

[ghost]

I didn't notice this, thanks.

[MartinNowak]

This proposal was also discussed in the newsgroup.

Yeah, I also thought about this.

This patch is a follow-up to a discussion in the newsgroup. The discussion is linked to in the corresponding Bugzilla issue.

I'm not sure how a DIP would be appropriate

I've read the discussion. The DIP is to condense/fixate the discussion outcome and to promote a proper design of a feature. Having a simple DIP is not a problem but as the discussion shows there are already many constraints and corner cases.

[ghost]

Well it's a start in some direction. As for compiler internals like importedFrom, this escapes me. Perhaps someone more knowledgeable with internals would pick up the work and go from there.

As for the feature itself, it's not controversial. The compiler already tracks all dependencies and finds everything via import switches. It seems only natural that it would have the ability to compile everything itself. But since the compiler is so hacky in many places it might require more than this simplistic pull.

[CyberShadow]

Having a simple DIP is not a problem but as the discussion shows there are already many constraints and corner cases.

These are not part of this pull request. As I said, that would be a separate enhancement.

I think the bureaucracy overhead is uncalled for in this case.

[dnadlinger]

Note aside: The module->importedFrom dependent template emission heuristics are a huge pain anyway (especially w.r.t incremental compilation, e.g. http://d.puremagic.com/issues/show_bug.cgi?id=8769, and several related LDC issues in the past).

[eskimor]

@klickverbot Indeed, I already have a patch ready laying out the foundation for a better mechanism.

[ghost]

I've had some odd crashes in the glue layer with this feature. I guess this isn't as simple to implement as it seemed it would be. Closing for now.

[CyberShadow]

I haven't encountered any problems while using this feature. Could the crashes be latent bugs? Have you tried reducing them?

[ghost]

@CyberShadow: I haven't reduced it yet, but the failing test-case can be seen in https://github.com/AndrejMitrovic/dgen in the BuildFail branch.

There's a build script in the src dir, it calls dmd -rb -m32 -Ilib dgen\main.d and fails with:

dgen\format\indent.d(29): Error: function dgen.format.indent.indentC
ode compiler error, parameter 'tokens', bugzilla 2962?
assert glue.c(817) 0

Building via RDMD works. Also building with DMD manually via the response file works (build.rsp).

[ghost]

Reopening this, it's rebased and there's no need to stall it. I'll go through all the comments again in the meantime.

[ghost]

Bad fixup in mars.c, fixing it now.

[ghost]

This is strange, I don't remember writing this section at all. I definitely wouldn't use these strange names for the variables or use lone ; in else statements or if right next to { braces. :s

[ghost]

We're using module->importedFrom in several places to attach template symbols and typeinfo
to an object file. If you're actually building the module then importedFrom will lose that semantic
and should be either set to the module itself or we need to fix all occurrences and use a better
mechanism.

Ah, I see the comment in the source here:

    bool isRoot() { return this->importedFrom == this; }
                                // true if the module source file is directly
                                // listed in command line.

I should probably set this->importedFrom = this; if a module ends up being built due to -rb.

Edit: Did just that.

[ghost]

Added default exclusion lists to the .ini/.conf files.

[ghost]

I don't think the tester uses those ini/conf files, I'm getting multiple definition errors.

[ghost]

Yeah, the -rxobject -rxstd.* -rxcore.* -rxetc.* flags in the ini files are not being picked up by the autotester, just tested locally.

[ghost]

As a workaround for the testsuite I've added -rxobject -rxstd.* -rxcore.* -rxetc.* in the shell script for the test-case.

[CyberShadow]

Does the test suite consult any ini files? If not, then that's probably the correct fix rather than a workaround.

[ghost]

Does the test suite consult any ini files?

I don't think it does, but I thought maybe the ini files in the ini folder were used by the autotester. Guess not.

[ghost]

Updated: Using -rx=foo syntax rather than -rxfoo, as it makes the call much more readable. Note that we don't have a convention in DMD, it's rather arbitrary what we pick. E.g.: -cov=nnn / -Dddocdir / -run srcfile. @CyberShadow agreed too.

[ghost]

I think Martin's idea about whitelists could be useful. E.g. if you want to build your library foolib you could be able to build it with a single command (if one module imports all the rest):

dmd -lib -rb foolib\package.d -rb=foolib.* -rx=*

Essentially this would build foolib.a with only the code in that library rather than accidentally building in code from some other library.

[CyberShadow]

Wait, what happens if you specify a module on the command line that is blacklisted? Does it error out, or does it only build that module? I think it should error out, since if it doesn't the result will most likely not be what the user expected.

[ghost]

@CyberShadow : I haven't thought of that. Yeah it should error out, will implement now.

[ghost]

Although.. I'm not sure. -rx is supposed to be about preventing built-in automatic recursive builds from building a module, and not preventing the user to override this. Hmm..

[CyberShadow]

What do you mean? When would this be useful?

[MartinNowak]

I still think this change goes in the wrong direction.
What this pull tries to solve is that rdmd needs 2 compilation passes.
Rather than integrating rdmd into dmd a much superior solution IMO is to have the compiler as a library and rdmd as a tool which does the recursive build using libdmd.
An intermediate solution to speed up rdmd might be desirable anyhow.

[CyberShadow]

Didn't think of that. How far is libdmd by current estimates?

[ghost]

Keep in mind this pull was initially made 10 months ago. DDMD is coming soon, so I agree "libifying" DMD could be an option in the near future. Hence maybe we should close this?

[ghost]

Clicked the wrong button.. but I guess we should close anywho.

[MartinNowak]

I'm a bit sceptical that this is sufficient. What if Module::load is called during semantic3, you'll call buildModules.push(m); with a module that had no semantic run yet. When I implemented a similar functionality (~2 years ago) I had to explicitly do that.

[MartinNowak]

Didn't think of that. How far is libdmd by current estimates?

Several month definitely, and it will be hard to do the necessary compiler refactorings, so it might take forever.
If we still want to solve this issue in the meanwhile we could add an under the hood options for rdmd only, e.g. add an undocumented -rdmdslave switch.
Then you'd no longer need an -rx switch or any filtering because rdmd already knows how to do that. You just need to get the compiler to ask rdmd using some kind of IPC.

[CyberShadow]

Well, I think the main goal for the new switches is to get rdmd to use them - the -rx switch maps 1:1 with rdmd's --exclude switch, so no IPC is necessary. So your suggestion is to make the switches longer and undocumented?

[MartinNowak]

No, my suggestion is to add an exclusive/private functionality for rdmd that we can change or drop whenever we want to.

[CyberShadow]

Yep, we're on the same page here. I was asking more about the means.

[MartinNowak]

So your suggestion is to make the switches longer and undocumented?

Mainly undocumented, longer might help still help to prevent usage.
How about -rdmd-build and -rdmd-exclude, i.e. prefix them with a rdmd namespace?

[timotheecour]

Module*m=