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 support for custom object module #689

Closed
wants to merge 4 commits into from
Closed

Add support for custom object module #689

wants to merge 4 commits into from

Conversation

ryuukk
Copy link
Contributor

@ryuukk ryuukk commented Oct 19, 2022

Fixes #688

image

Copy link
Member

@WebFreak001 WebFreak001 left a comment

Choose a reason for hiding this comment

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

is object any special? is this PR really needed? Code looks like if you just add your custom source path with object.d before the stdlib path it should simply work.

It rather looks like -I should prepend imports or offer some option to prepend them - or built-in imports being moved to the end.

Your code looks like it's gonna import all the object.d files that exist, does that conform to the spec? I thought a custom object.d file would override the built-in one, not merge with it.

@ryuukk
Copy link
Contributor Author

ryuukk commented Oct 19, 2022

by default object.d is implicitly imported in every modules

you can override this module by creating an object.d in your project, that's how one create a custom runtime: https://github.com/adamdruppe/webassembly/blob/master/arsd-webassembly/object.d

the current DCD behavior is to just pick the first one to come, if you still use the druntime, but still want to override object.d, then DCD will always pick the one from druntime, since import path for druntime is always provided first

i decided against filtering, because of issues like this: Pure-D/serve-d#260

so it's best to have a safe behavior, custom object.d is a niche use case anyways, so have both code completion for default object.d and the custom one is better than having just the default one

@WebFreak001
Copy link
Member

WebFreak001 commented Oct 20, 2022

I agree custom object.d is a niche use-case, but we want to support it properly anyway, because there are quite a few valid use-cases which also have been used in the community. (e.g. webassembly druntime, lwdr, etc)

So instead of duplicating a lot of the import lookup code just for the implicit import object, which should not be any special, we should fix the ordering of the import paths. The same issue applies to all of phobos as well: if you define a custom std/stdio.d, it should override phobos, and does so with dmd, but DCD will still auto-complete from phobos instead. And the merging thing doesn't work that well there.

@ryuukk ryuukk closed this by deleting the head repository Feb 14, 2023
@ryuukk
Copy link
Contributor Author

ryuukk commented Jan 21, 2024

I think this PR should be revived, i work exclusively with a custom runtime (wasm), and not having completions for my type is kinda annoying

@WebFreak001
Copy link
Member

WebFreak001 commented Jan 21, 2024

I reevaluated this and it looks like what DCD is doing already is correct and your problem is DCD misconfiguration from the IDE. However only being able to add import paths to the end, where they have less precedence seems to make this unnecessarily complicated. In the compiler the first specified import also has highest precedence, but all the built-in imports from the compiler are always appended to the end and not to the start, like what DCD does.

Since DCD does not have any notation of what is a built-in import from the compiler and relies on the IDE for all imports (or external user-configuration that probably shouldn't be loaded in an IDE environment), the order is completely dependent on the IDE and the API that DCD currently exposes to modify them is quite cumbersome for IDE developers.

What an IDE should do to support this properly is:

  • maintain the current list of imports
  • on change:
    • remove all the imports after where a new import would be inserted
    • add the new imports
    • re-add all removed imports again

It'd probably be a good idea to make a simpler DCD command for that for IDEs, e.g. --allImports which replaces the internal import list completely.

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.

Custom object.d should be included by default
2 participants