GC Config: initialization method does not need access to the outer scope

[denizzzka]

I.e., Config isn't need to touch outer _initialized directly

Having such access prohibits replaceable configuration structs

[thewilsonator]

also would making _initialized a static member of Config help for what you want to do here?

[thewilsonator]

marking this package(core) means regular users cannot call this function. Is this intended?

[denizzzka]

Yes: by the time regular users can call it it's already been called and tryToInitialize() won't be executed a second time because _initialized == true

[denizzzka]

I made it private here because method name changed anyway.
If we need to be able initialize it somewhere externally (a second instance of Config, for example) we can change that in the future. But I think this is not needed

[denizzzka]

@thewilsonator

also would making _initialized a static member of Config help for what you want to do here?

This is impossible, because adding static members to Config breaks compile-time directive:

src/core/internal/parseoptions.d(101): Error: variable `cfg` cannot be read at compile time
                        static if (hasUDA!(__traits(getMember, cfg, field), MemVal))

(Besides, I think it is architecturally correct to do it as I suggested in this PR)

[denizzzka]

@schveiguy Hi! Tell your opinion about this PR?

[schveiguy]

Having such access prohibits replaceable configuration structs

Can you elaborate on this? I.e. what is the problem this PR is fixing.

[denizzzka]

Can you elaborate on this? I.e. what is the problem this PR is fixing.

I'm planning to add the ability to use other config structs (literally a different structs with the same field names) declared in other modules. This is necessary to be able to replace defaults, implement other ways to configure or remove configurability altogether because this is not necessary everywhere and may be incompatible with baremetal targets. But this access from struct itself to external _initialized prevents this.

[schveiguy]

So is there a further plan to accept a config struct as a template parameter? I don't see how this change enables using a different config struct.

[denizzzka]

So is there a further plan to accept a config struct as a template parameter?

I don't know yet, maybe. I used to make the switching by static if previously.

I don't see how this change enables using a different config struct.

Structure from another module haven't access to _initialized variable

[schveiguy]

OK, but if you are are editing druntime to effect your fix anyway, then why not just make this change locally? And then when you want to make the change to druntime to allow alternative configuration structs, then you make this change at the same time.

I'm not saying we don't want to do this. What I'm saying is that this change by itself doesn't seem to give you any ability to change the Config structure to something else. So I'm not sure we need it by itself.

Structure from another module haven't access to _initialized variable

It still does not.

[denizzzka]

And then when you want to make the change to druntime to allow alternative configuration structs, then you make this change at the same time.

Yes, I just decided to state my intentions: seems, there will be a lot tied to this change

It still does not.

If this PR is accepted, access to this variable will become unnecessary

[schveiguy]

It's just that this change doesn't enable anything except a different editing of druntime.

What about a way to replace the tryToInitialize function with a custom one? Maybe via a weak symbol?

Something similar to

dmd/druntime/src/core/internal/gc/proxy.d

Line 42 in f047a03

void* register_default_gcs() @weak

I'd suggest something like this:

@weak extern(C) bool initialize_gc_options(Config *config) {
    return config.tryToInitialize();
}

void initialize(ref Config config) {
    if (!_initialized)
        _initialized = initialize_gc_options(&config);

    return _initialized;
}

And then your change makes sense. Because now you can replace that call with your own. See if something like this works with your changes that you are trying (and makes it so you don't have to edit druntime).

[denizzzka]

In my PR I can also replace tryToInitialize() struct member with my own if I create my own config struct, only without "magic" of weak symbols