Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

move modules necessary to implement a GC to core.gc#2491

Merged
dlang-bot merged 2 commits intodlang:masterfrom
rainers:core_gc
Mar 4, 2019
Merged

move modules necessary to implement a GC to core.gc#2491
dlang-bot merged 2 commits intodlang:masterfrom
rainers:core_gc

Conversation

@rainers
Copy link
Copy Markdown
Member

@rainers rainers commented Feb 15, 2019

This allows compiling new GC implementations without requiring adding an import path to druntime/src.

Not sure if this is better than just adding the 3 gc-files to the import folder as is. Comments?

@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request, @rainers!

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 + druntime#2491"


import gc.config;
import gc.gcinterface;
import core.gc.gcinterface;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Couldn't this file import the declarations from gc.proxy instead of defining the headers again?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure what you mean. Do you prefer a cyclic dependency and a public import in gc.proxy over a simple import of the public module?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you prefer a cyclic dependency

Ah sorry. Nevermind then.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, I didn't see the gc_* declarations in the diff. I think we have these declarations elsewhere too, e.g. core.memory. IIRC this is done to fake some attributes.

*/

module gc.config;
module core.gc.config;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we want to expose this to the user and docs? Maybe make everything package for now?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think a GC implementation should be able to use the configuration even if it does not support all the values in there.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe make everything package for now?

That would disallow accessing these modules from package gc AFAICT.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That would disallow accessing these modules from package gc AFAICT.

Fair enough.

@dlang-bot dlang-bot merged commit 0a3edfd into dlang:master Mar 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants