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

Fix issue 10023 - Add rtInfo (or equivalent) to ModuleInfo. #2271

Closed
wants to merge 1 commit into from

Conversation

jacob-carlborg
Copy link
Contributor

No description provided.

@jacob-carlborg
Copy link
Contributor Author

Don't know if someone else can come up with a better name.

@jacob-carlborg
Copy link
Contributor Author

dlang/druntime#534 is the runtime part needed to complete this.

@schveiguy
Copy link
Member

Why does it have to be RMInfo, can't RTInfo be used for both? The parameter passed into it is a module, not a type, should be pretty straightforward to overload, no?

@MartinNowak
Copy link
Member

What I really don't like about these is that this is not user-data but runtime data, as you can only define RTInfo/RMInfo in object_.di.
While this made sense to experiment with precise GC information, the template should be instantiated in the module scope for user-data.

Why does it have to be RMInfo, can't RTInfo be used for both?

Probably, there is also getMembers which does something similar.
Please clarify the purpose and scope of this in more detail (Bugzilla).

@jacob-carlborg
Copy link
Contributor Author

@schveiguy It was easy to do like this. It's straightforward in D but I have no idea how to implement that in the compiler.

@dawgfoto I agree. There's a problem that it only works in object.di. I think we should have some mechanism that makes it possible for RTInfo/RMInfo to be used outside out object.di.

@jacob-carlborg
Copy link
Contributor Author

I have an idea:

  1. We add a new struct (rtIfno/rmInfo) to object.di as an UDA recognized by the compiler
  2. If a template that have the correct signature and the object.rtInfo UDA attached, instantiated that as RTInfo/RMInfo
  3. Collect the result of the RTInfo/RMInfo template in an associative array. The keys will be the fully qualified name of the module the template is defined in, the values will be what the same as now

@jacob-carlborg
Copy link
Contributor Author

Any comments?

@MartinNowak
Copy link
Member

I think a simpler approach would be to instantiate rtInfo/rmInfo from the scope of the type/module, thus allowing to define or import a different implementation.
Configurability doesn't make sense for precise-GC metadata though.

What about getMembers, it serves almost the same purpose?

@schveiguy
Copy link
Member

Sorry, I forgot about this.

I think storing as an AA is making things overly complex. Why can't rtInfo simply have directly accessible fields?

@dawgfoto's approach seems the most sane, give the power to the template to do whatever it wants in the correct context, including generating an AA.

What about getMembers, it serves almost the same purpose?

The idea is that we can generate at runtime the information that we need for various runtime algorithms, such as precise GC, named unit tests, or better module cycle detection. These are the three I had in mind. The idea behind rtInfo is that it generates runtime accessible information, vs. getMembers which accesses compile-time accessible information.

@jacob-carlborg
Copy link
Contributor Author

@dawgfoto

I think a simpler approach would be to instantiate rtInfo/rmInfo from the scope of the type/module, thus allowing to define or import a different implementation.

I don't think I understand.

@jacob-carlborg
Copy link
Contributor Author

@schveiguy

I think storing as an AA is making things overly complex. Why can't rtInfo simply have directly accessible fields?

Currently RTInfo is limited to object.di. My idea was that you can put an RTInfo template in any module to be able to have multiple implementations and not have to modify object.di to add new information to RTInfo. Currently there is one instance of RTInfo for each TypeInfo, that works fine since you can only have RTInfo in object.di. With my approach the rtInfo field would be an AA, the keys would be the fully qualified name of the module where the RTInfo template is defined in. The values would be void* like now.

Example, how it currently works:

module object;

template RTInfo (T)
{
    enum RTInfo = collectInfo!(T);
}
module foo.bar;

struct Foo {}
auto info = typeid(Foo).rtInfo;

Example of my approach:

module foo.bar;

struct Foo {}

@rtInfo template RTInfo (T)
{
    enum RTInfo = collectInfo!(T);
}

auto info = typeid(Foo).rtInfo["foo.bar"];

@MartinNowak
Copy link
Member

With my approach the rtInfo field would be an AA, the keys would be the fully qualified name of the module where the RTInfo template is defined in.

That would not really work with separate compilation, right?

I don't think I understand.

Just instantiating the templates at the scope (location) of the Type definition.
So if you define or import rtInfo in the same module it will be used. I think this approach would be better than having a single global definition.

@jacob-carlborg
Copy link
Contributor Author

That would not really work with separate compilation, right?

What would the difference be compared to now. What happens now with separate compilation?

Just instantiating the templates at the scope (location) of the Type definition.
So if you define or import rtInfo in the same module it will be used. I think this approach would be better than having a single global definition.

Sorry, I still don't think I understand. Could you show an example? My approach does not have a single global definition. My approach can have a definition per module (I don't think any more is needed), that's why the AA is necessary.

@MartinNowak
Copy link
Member

What happens now with separate compilation?

module a;
@rtInfo template RTInfo (T) { enum RTInfo = 0; }
struct Foo {}
assert(typeinfo(Foo).rtInfo["a"] == 0);
// This wouldn't work, because the compiler doesn't know about module b
assert(typeinfo(Foo).rtInfo["b"] == 1);
module b;
@rtInfo template RTInfo (T) { enum RTInfo = 1; }

Sorry, I still don't think I understand.

Basically it works like this.

struct Foo {}
// compiler instantiates the unqualified `rtInfo` from the point of definition
TypeInfo_Struct.rtInfo = rtInfo!Foo;

// Would work with imports
import gc : rtInfo;
// Would work with definitions
template rtInfo(T) { enum rtInfo = 0; }
// Would work with AA's (if they were CTFEable)
template rtInfo(T) { enum rtInfo = buildAA!(gc.rtInfo, .rtInfo)(); }

@jacob-carlborg
Copy link
Contributor Author

Basically it works like this.

I'm still a bit confused.

compiler instantiates the unqualified rtInfo from the point of definition

Do you mean I have to define (or import) rtInfo in the same module as the type I want to collect information about? If that's the case, I think it defeats a big part of rtInfo.

@MartinNowak
Copy link
Member

Do you mean I have to define (or import) rtInfo in the same module as the type I want to collect information about? If that's the case, I think it defeats a big part of rtInfo.

Of course, how would the compiler know about rtInfo otherwise?

@@ -196,6 +203,7 @@ void Module::genmoduleinfo()
void *sharedctor;
void *shareddtor;
uint index;
void *rmInfo;
void*[1] reserved;
Copy link
Member

Choose a reason for hiding this comment

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

Why keep around void*[1] reserved if you're adding a new field that takes up that space?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason. I'll remove it.

Copy link
Member

Choose a reason for hiding this comment

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

Just a note that the reserved field brings the size of ModuleInfo is 64 bytes (128 on 64bit) - there's probably am reason why it was padded up to a power of 2. Though can't think why at this moment. :)

@jacob-carlborg
Copy link
Contributor Author

Of course, how would the compiler know about rtInfo otherwise?

My idea was to have a UDA that the compiler recognizes.

@MartinNowak
Copy link
Member

My idea was to have a UDA that the compiler recognizes.

You would still have to import or define the template.

@jacob-carlborg
Copy link
Contributor Author

You would still have to import or define the template.

Yes, but I can define it anywhere. I can inspect third party types.

@MartinNowak
Copy link
Member

Yes, but I can define it anywhere. I can inspect third party types.

You mean something like this should work?

module a;
struct Foo {}
module b;
import a;
template rtInfo(T) { enum rtInfo = 0; }
assert(typeid(Foo).rtInfo["b"]);

This is problematic because the TypeInfo for Foo is generated when a is compiled. Thus the compiler does not know b. To make it work you'd need to fill the AAs at runtime.
I'm not really convinced that this would be a good default mechanism.
If you still wanted to use it you could define rtInfo as template rtInfo(T) { enum rtInfo = cast(void*[string])null; } and add static constructors.

@jacob-carlborg
Copy link
Contributor Author

You mean something like this should work?

module a;
struct Foo {}
module b;
import a;
template rtInfo(T) { enum rtInfo = 0; }
assert(typeid(Foo).rtInfo["b"]);

Yes, that was the idea.

This is problematic because the TypeInfo for Foo is generated when a is compiled. Thus the compiler does not know b. To make it work you'd need to fill the AAs at runtime. I'm not really convinced that this would be a good default mechanism.

Ok, I see. The reason for the AA is to support multiple rtInfo templates. In that case the users would need to know where the rtInfo data comes from.

Is there some other way that we can support what I want to achieve?

@jacob-carlborg
Copy link
Contributor Author

@MartinNowak bringing up this discussion again. I feel we couldn't really agree last time about the ModuleInfo stuff. How about I change the pull request to only implement the template part? That is still very useful. Then we can take care of the ModuleInfo some other time.

@MartinNowak
Copy link
Member

Sorry, I don't really have time for this right now. I'd prefer if you discussed the details with @yglukhov about the nested rtinfo idea and this PR and come up with a combined solution.

Is there some other way that we can support what I want to achieve?

You cannot inject arbitrary code/data into foreign modules without changing them.

@yglukhov
Copy link
Contributor

Ahhh, just noticed this PR, after i've done something related here.
dlang/druntime#775
Sorry for duplication.

@jacob-carlborg
Copy link
Contributor Author

@MartinNowak fair enough.

@quickfur
Copy link
Member

ping

@jacob-carlborg
Copy link
Contributor Author

It seems @MartinNowak wants to solve the issue with custom RTInfo/RMInfo first, before tackling this issue since it will be affected. See: dlang/druntime#775.

@andralex
Copy link
Member

Status of this?

@jacob-carlborg
Copy link
Contributor Author

Status of this?

The status hasn't changed since my last post: #2271 (comment). The quest is, what is the status of dlang/druntime#775. It's mostly @MartinNowak that had objections to this and he doesn't have time.

I can update the code and make sure it can be merged and passes the tests. But that seems pointless to me if there's objections to the concept.

See also: dlang/druntime#775.

@MartinNowak
Copy link
Member

I'll close this, as this is as pointless as RTInfo.
We need to come up with a workable solution that allows to customize rtInfo/rmInfo without changing object.di.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants