Document __traits(isModule) and __traits(isPackage)

[blm768]

This PR documents the language changes in dlang/dmd#5290.

[dlang-bot]

Thanks for your pull request and interest in making D better, @blm768! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

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.

[JinShil]

I noticed when review the DMD PR that if a module is not imported, that __traits(isModule) can still resolve it. That's a pretty important condition, IMO, and I think it should be spelled out in the the specification.

[wilzbach]

How about defining more explicitly what a module symbol and package symbol is?
The examples show it a bit, but I think the specification should be a bit more specific than just relying on examples.

[blm768]

@wilzbach I've made the wording a bit less ambiguous, but let me know if you think we should expand more on what's there.

@JinShil Does that apply just to modules that have been imported during the current compiler run but not in the current module? I thought that the logic in isPkgX (traits.d, line 527) wouldn't return true for symbols that have never been imported, but it's been two years since I first wrote the PR, so my mental model could be out of date. Or are you referring to how Package.resolvePKGUnknown will check for a package.d even when the package has never been imported as a module?

[wilzbach]

I would add another sentence here - defining what a package is. Maybe sth. like this?

A package is a grouping of modules. package.d modules are considered as a package, however the existence of a package.d file for a package isn't required.

[JinShil]

Does that apply just to modules that have been imported during the current compiler run but not in the current module? I thought that the logic in isPkgX (traits.d, line 527) wouldn't return true for symbols that have never been imported, but it's been two years since I first wrote the PR, so my mental model could be out of date. Or are you referring to how Package.resolvePKGUnknown will check for a package.d even when the package has never been imported as a module?

I'm referring to this: https://github.com/blm768/dmd/blob/381813d646c23cabbb043a0f1b722d707a232f21/test/runnable/traits.d#L629-L630

Just ensure the documentation clearly explains how the implementation works so the reader can accurately predict the behavior simply reading the documentation, without the need to write a test to figure it out.

[blm768]

@wilzbach Do you think linking to the module documentation provides enough explanation of packages? It seems a little less redundant than fully explaining them in traits.dd, but they're not terribly prominent in module.dd, either.

@JinShil I'd almost forgotten that comment was there. I think the note I've added for isModule should cover that case now.

[JinShil]

What about this?

import std.algorithm;

void fun()
{
    // `std.stdio` was not imported into this file, but it is in the import path.
    // Does this return `true` or `false`?
    static assert(!__traits(isModule, std.stdio));
}

[adamdruppe]

On Wed, Feb 14, 2018 at 11:47:51PM +0000, Mike Franklin wrote: // `std.stdio` was not imported into this file, but it is in the import path. // Does this return `true` or `false`? static assert(!__traits(isModule, std.stdio));

so I haven't been following this discussion, but I think the answer is "neither"... it should be a compile error, undefined symbol, std.stdio. I had an argument on IRC yesterday about is() (well, not so much argument, more like "violent agreement") and how it swallows errors and that causes a lot of problems for D's users because it returns false on typos when it should just say "T is undefined". We might or might not change is(), but we should definitely not introduce more swallowed errors where we don't need to. The isModule trait should require an in-scope name already to even be called, otherwise it should be an error. And since std.stdio is not imported in this module, the name is not in scope.

[blm768]

The intended behavior is for undefined symbols to produce an error, and that's the result which a quick test on my machine gives. There's some special logic to check whether a package (which will be in scope but might not be directly imported) has a package.d, but otherwise the code falls back on the normal symbol resolution mechanics.

[thewilsonator]

Ping me when this is good.

[blm768]

@thewilsonator I think I've covered pretty much everything that's relevant now. Do you think it's worth having any cross-linking between the __traits and is() entries?

[thewilsonator]

Do you think it's worth having any cross-linking between the __traits and is() entries?

Possibly? I haven't really thought about it. Regardless, it need't hold up merging this. Feel free to do it in another PR if you think it would be useful.