New __traits: isModule and isPackage #5290

blm768 · 2015-12-01T03:08:34Z

This pull request would add two new __traits: isModule and isPackage, which (unsurprisingly) allow one to determine whether a symbol represents a module and/or a package. These traits can be quite useful for introspection under certain circumstances, especially some types of code generation (i.e. bindings).

Spec change: dlang/dlang.org#2199

9rnsr · 2015-12-01T12:30:17Z

src/traits.d

+                    //TODO: use std.format instead once GDC's Phobos allows it?
+                    import std.string;
+                    assert(imp.pkg,
+                        format("unable to process forward-referenced import %s", imp.toChars()));


Don't use Phobos utilities in dmd code.

Makes sense. What does DMD normally use for string concatenation? Should I just use strcat?

printf followed by assert(0), inside an if.

That should work. I'll probably use fprintf(stderr, ...), though.

Better to follow the existing style, which prints to stdout.

Hmm. That's a little odd. I guess it works, though. Fixed.

yebblies · 2015-12-02T01:57:20Z

test/runnable/traits.d

@@ -614,6 +614,21 @@ void test23()
   assert(toString23(w) == "alembicated");
 }

+void test24() {
+    import core.atomic;
+    import std.algorithm;


We try not to import phobos inside the dmd test suite either.

I was looking for something that had some "plain" packages plus modules and "package.d", but I suppose a cleaner solution would be to make a module in test/runnable/imports. I'll fix that.

ibuclaw · 2015-12-02T06:57:26Z

src/traits.d

+                {
+                    if (!imp.pkg)
+                    {
+                        printf("unable to process forward-referenced import %s", imp.toChars());


error("Internal Compiler Error: your message %s", imp.toChars()); ?

That might work. I thought the error() function needed a Loc object, though. I'm not sure how to get one in this context.

Edit: oh, duh. e.loc should work. I think...

Or just Loc() for no location.

blm768 · 2015-12-10T07:35:51Z

I may have rebased this a bit prematurely, but it's rebased to master now.

andralex · 2016-01-15T03:40:14Z

Looks meaningful. @WalterBright ?

blm768 · 2016-02-12T19:53:57Z

Fixed the merge conflict and rebased again.

blm768 · 2016-03-23T19:14:14Z

Is approval by @WalterBright the only thing holding this PR back? There's no rush; I just want to make sure there's nothing I need to fix. (Well, and bump this PR so it doesn't get buried forever ;)...)

AndrejMitrovic · 2016-05-06T21:29:40Z

These traits can be quite useful for introspection under certain circumstances, especially some types of code generation (i.e. bindings).

Could you show some real-world use-cases? Also, an associated bugzilla entry should be linked to (create one if it doesn't exist). Thanks.

blm768 · 2016-05-07T22:37:02Z

Could you show some real-world use-cases? Also, an associated bugzilla entry should be linked to (create one if it doesn't exist). Thanks.

The use case that prompted this was the refactoring of my old Bullet Physics bindings. Originally, it used a kludgy build process that looped over the source files and created a new D program that would iterate over all the packages and pull out relevant symbols that needed "glue" code to interact with C++. Being able to iterate over the packages directly at compile time would be a fair bit cleaner. With the improved C++ interop that has developed since I started this PR, it's probably not as relevant as it used to be, but I can see it still being useful for binding to other languages.

I'll create a Bugzilla entry. Thanks for pointing that out.

EDIT: here's the Bugzilla entry.

AndrejMitrovic · 2016-05-08T08:16:49Z

Pretty solid use request! :)

I'll take a look in the front-end to see if we can use a hashmap of some kind to avoid O(n) lookups.

blm768 · 2016-05-17T17:06:20Z

I'll take a look in the front-end to see if we can use a hashmap of some kind to avoid O(n) lookups.

So basically a __traits(allPackages)?

AndrejMitrovic · 2016-05-17T20:24:28Z

Sorry, I thought you introduced a linear lookup somewhere, I might have been wrong here.

blm768 · 2016-05-17T20:44:16Z

Sorry, I thought you introduced a linear lookup somewhere, I might have been wrong here.

It should mostly be constant-time in this code. resolvPkgUnknown is linear with respect to the nesting depth of the package when it builds the identifier chain to try to find a package.d, but that's dwarfed by the overhead of the filesystem access that it does. Any iteration would be done by the user of the traits, who would (at least under certain circumstances) iterate over symbols and figure out which ones are packages.

AndrejMitrovic · 2016-05-17T20:55:23Z

Anyway I don't find this is a feature that needs a whole lot of discussion, it's pretty simple. Could you rebase it against master?

blm768 · 2016-05-18T16:18:15Z

Done. Now we'll see if the auto-tester likes it...

AndrejMitrovic · 2016-05-18T21:44:00Z

@WalterBright @andralex: this is a pretty low-impact feature request, can we get an a-ok for adding the two new traits? It would benefit the BulletD project (which is high on the list of wanted C++ bindings, right up there with QtD).

AndrejMitrovic · 2016-05-18T21:46:03Z

@blm768: P.S. you're going to have to squash your commits. :)

But let's wait for W&A's approval first and then I'll have another look at the code and merge it.

lodo1995 · 2016-07-18T21:05:47Z

What's the status of this? It would be very useful.
For example, I'm currently trying to write some code that looks for all available templates marked with a specific UDA. To do this, I need to enumerate all visible symbols and if a symbol is an imported module or package I need to enumerate its members. These two traits would be very useful in this case.

blm768 · 2016-07-23T18:01:52Z

Just rebased again and squashed.

blm768 · 2019-03-16T20:37:28Z

Managed to find and fix another off-by-one error in my changes. Apparently my D is rusty or something.

Anyway, it looks like something in my changes has broken some aspect of scope resolution inside the is() expression or something. The following minimal example is producing an error:

module test.compilable.test16002;
import imports.plainpackage.plainmodule;
static assert(!is(imports.plainpackage.plainmodule == package));

Error: module `imports.plainpackage.plainmodule` from file test/compilable/imports/plainpackage/plainmodule.d must be imported with 'import imports.plainpackage.plainmodule;'
test/compilable/test16002.d(13):        while evaluating: static assert(!false)

I haven't managed to determine why yet, but what seems to be happening is that the result of e.targ.toDsymbol is a Package with isPkgMod == PKG.unknown (even though it's just a plain module inside a package), which is causing a call to resolvePKGUnknown, which locates the correct filename but then raises the error message in Module.load. It doesn't seem to make any difference whether I use the original Scope object (sc) or the modified one that the IsExp visitor uses for types (sc2)

thewilsonator · 2019-05-11T13:01:23Z

It was decided to have both.

blm768 · 2019-05-11T20:42:01Z

@thewilsonator: Both the __traits and is()? Or are you referring to something related to PGK.unknown resolution?

thewilsonator · 2019-05-12T08:00:45Z

Both __traits and is()

thewilsonator · 2019-05-12T08:01:56Z

(I'm about to travel back home so it will be probably 2 days before I get back around to this. Do ping me if I forget!)

test/fail_compilation/traits.d

src/dmd/traits.d

thewilsonator · 2019-05-19T06:47:17Z

Thanks, please squash the commits and this is good to go.

thewilsonator · 2019-05-19T06:51:49Z

==============================
Test fail_compilation/traits.d failed: 
expected:
----
fail_compilation/traits.d(100): Error: `getTargetInfo` key `"not_a_target_info"` not supported by this implementation
fail_compilation/traits.d(101): Error: string expected as argument of __traits `getTargetInfo` instead of `100`
fail_compilation/traits.d(102): Error: expected 1 arguments for `getTargetInfo` but had 2
fail_compilation/traits.d(103): Error: expected 1 arguments for `getTargetInfo` but had 0
fail_compilation/traits.d(200): Error: undefined identifier `imports.nonexistent`
fail_compilation/traits.d(201): Error: undefined identifier `imports.nonexistent`
fail_compilation/traits.d(200): Error: undefined identifier imports.nonexistent
fail_compilation/traits.d(201): Error: undefined identifier imports.nonexistent
fail_compilation/traits.d(202): Error: expected 1 arguments for `isPackage` but had 0
fail_compilation/traits.d(203): Error: expected 1 arguments for `isModule` but had 0
----
actual:
----
fail_compilation/traits.d(100): Error: `getTargetInfo` key `"not_a_target_info"` not supported by this implementation
fail_compilation/traits.d(101): Error: string expected as argument of __traits `getTargetInfo` instead of `100`
fail_compilation/traits.d(102): Error: expected 1 arguments for `getTargetInfo` but had 2
fail_compilation/traits.d(103): Error: expected 1 arguments for `getTargetInfo` but had 0
fail_compilation/traits.d(200): Error: undefined identifier `imports.nonexistent`
fail_compilation/traits.d(201): Error: undefined identifier `imports.nonexistent`
fail_compilation/traits.d(202): Error: expected 1 arguments for `isPackage` but had 0
fail_compilation/traits.d(203): Error: expected 1 arguments for `isModule` but had 0
----

blm768 · 2019-05-19T06:57:40Z

==============================
Test fail_compilation/traits.d failed: 
...

Yep. With my current level of consciousness, that was about guaranteed to happen. I really shouldn't be allowed to code at this time of night.

thewilsonator · 2019-05-19T07:01:43Z

dmd/expressionsem.d(2321): Warning: Ddoc: parameter count mismatch, expected 1, got 0
dmd/expressionsem.d(2321):        Note that the format is `param = description`

thewilsonator · 2019-05-19T07:02:59Z

Test fail_compilation/test16002.d failed: 
expected:
----
fail_compilation/test16002.d(100): Error: undefined identifier `imports.nonexistent`
fail_compilation/test16002.d(100):        while evaluating: `static assert(is(imports.nonexistent == package))`
fail_compilation/test16002.d(101): Error: undefined identifier `imports.nonexistent`
fail_compilation/test16002.d(101):        while evaluating: `static assert(is(imports.nonexistent == module))`
----
actual:
----
fail_compilation/test16002.d(100): Error: undefined identifier `imports.nonexistent`
fail_compilation/test16002.d(101): Error: undefined identifier `imports.nonexistent`
----

blm768 · 2019-05-19T07:09:38Z

Sorry; I'm really hammering those poor CI servers with all these failed attempts. Thanks for your patience while I fumble through this.

thewilsonator · 2019-05-19T07:11:20Z

No problems, I hammer them frequently too. That you for your continued interest!

blm768 · 2019-05-19T07:24:35Z

Well, I figured I might as well get this finished after three years. ;)

I'll try to get the changes to the spec hammered out in the next couple of days.

thewilsonator · 2019-05-19T07:25:27Z

Haha, thanks.

ntrel · 2023-02-25T12:15:47Z

@thewilsonator

It was decided to have both.
Both __traits and is()

Why was that? is should only be true when its argument is a valid type - this is important conceptually. A module is not a type. I think we should deprecate is(m == module), is(p == package) for this reason.

thewilsonator · 2023-02-25T12:24:32Z

I don't remember, this was 4 years ago.

I think we should deprecate is(m == module), is(p == package) for this reason.
I don't think so, is(foo == keyword) is different from is(foo == type).
There is no practical utility in deprecating it: it can have no other meaning, it does not compete for design space with anything.

9rnsr reviewed Dec 1, 2015
View reviewed changes

blm768 force-pushed the new_traits branch from 8b378a7 to 006d1c4 Compare December 2, 2015 01:32

yebblies reviewed Dec 2, 2015
View reviewed changes

blm768 force-pushed the new_traits branch from df66e6f to 46ad0c8 Compare December 2, 2015 02:32

ibuclaw reviewed Dec 2, 2015
View reviewed changes

blm768 force-pushed the new_traits branch from ee3ddb3 to 5033f30 Compare December 10, 2015 07:25

blm768 force-pushed the new_traits branch from 9622a5f to e0e3224 Compare February 12, 2016 19:26

blm768 force-pushed the new_traits branch from e0e3224 to 7b55647 Compare May 18, 2016 16:16

blm768 mentioned this pull request May 25, 2016

What's the state of this project? blm768/BulletD#3

Open

blm768 force-pushed the new_traits branch 2 times, most recently from 34e275c to bd60620 Compare July 23, 2016 18:01

blm768 force-pushed the new_traits branch from bd60620 to 98bdac7 Compare August 4, 2016 01:54

blm768 force-pushed the new_traits branch 2 times, most recently from ab065f4 to 6159360 Compare March 10, 2019 19:40

blm768 force-pushed the new_traits branch 3 times, most recently from 6a803ec to 44c6685 Compare May 18, 2019 06:47

thewilsonator reviewed May 19, 2019

View reviewed changes

test/fail_compilation/traits.d Outdated Show resolved Hide resolved

thewilsonator reviewed May 19, 2019

View reviewed changes

src/dmd/traits.d Show resolved Hide resolved

thewilsonator reviewed May 19, 2019

View reviewed changes

src/dmd/traits.d Show resolved Hide resolved

fix Issue 16002 - Add __traits(isModule) and __traits(isPackage)

01ff106

blm768 force-pushed the new_traits branch from c084868 to 01ff106 Compare May 19, 2019 07:04

thewilsonator added the Merge:auto-merge label May 19, 2019

dlang-bot merged commit d6158b4 into dlang:master May 19, 2019

pbackus mentioned this pull request Mar 30, 2022

Audit Log File #12710

Closed

New __traits: isModule and isPackage #5290

New __traits: isModule and isPackage #5290

Conversation

blm768 commented Dec 1, 2015 • edited by JinShil Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blm768 commented Dec 10, 2015

andralex commented Jan 15, 2016

blm768 commented Feb 12, 2016

blm768 commented Mar 23, 2016

AndrejMitrovic commented May 6, 2016

blm768 commented May 7, 2016 • edited Loading

AndrejMitrovic commented May 8, 2016

blm768 commented May 17, 2016

AndrejMitrovic commented May 17, 2016

blm768 commented May 17, 2016

AndrejMitrovic commented May 17, 2016

blm768 commented May 18, 2016

AndrejMitrovic commented May 18, 2016

AndrejMitrovic commented May 18, 2016

lodo1995 commented Jul 18, 2016

blm768 commented Jul 23, 2016

blm768 commented Mar 16, 2019

thewilsonator commented May 11, 2019

blm768 commented May 11, 2019

thewilsonator commented May 12, 2019

thewilsonator commented May 12, 2019

thewilsonator commented May 19, 2019

thewilsonator commented May 19, 2019

blm768 commented May 19, 2019 • edited Loading

thewilsonator commented May 19, 2019

thewilsonator commented May 19, 2019

blm768 commented May 19, 2019

thewilsonator commented May 19, 2019

blm768 commented May 19, 2019

thewilsonator commented May 19, 2019

ntrel commented Feb 25, 2023

thewilsonator commented Feb 25, 2023

blm768 commented Dec 1, 2015 •

edited by JinShil

Loading

blm768 commented May 7, 2016 •

edited

Loading

blm768 commented May 19, 2019 •

edited

Loading