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

Deprecate the usage of shared static this for initialization #2370

Open
Geod24 opened this issue Sep 24, 2019 · 4 comments
Open

Deprecate the usage of shared static this for initialization #2370

Geod24 opened this issue Sep 24, 2019 · 4 comments

Comments

@Geod24
Copy link
Contributor

Geod24 commented Sep 24, 2019

I think experience has shown that shared static this for initialization causes many problems.

The root of the cause is IMO that (shared) module constructors in D are second-class citizens, and are best used only for module-specific initialization. And while circular imports are well supported in D, it is only thanks to the fact that the compiler is involved and can figure out dependencies on the symbol level.

E.g. something as trivial as:

--- a.d
import b;
shared int v;
shared static this () { v = 42; }
void main () {}
--- b.d
import a;
shared int x;
shared static this () { x = 42; }

Will result in:

$ dmd b.d -run a.d
object.Error@src/rt/minfo.d(371): Cyclic dependency between module constructors/destructors of a and b
a* ->
b* ->
a*

Additionally, some symbols might not be ready, for example see Phobos issue 17564, order might be unpredictable among build tools / configurations and various level of scoping, for example #371 , and the original convenience is lost to the fact that one needs to define VibeDefaultMain in the dub file nowadays.

Thus, I propose to gradually deprecate the use of them.
The first step in doing so would be to remove any piece of documentation mentioning it.
Besides that, there doesn't seem to be too much specific code geared toward supporting it. One thing one could do is issue a message if certain (common) routines are called before main (e.g. listen{TCP,HTTP}, runTask, timers...).

@s-ludwig Does it fit into your vision for Vibe.d ?

@Geod24
Copy link
Contributor Author

Geod24 commented Aug 24, 2020

@s-ludwig : This is itching me. I'm thinking of starting the (quite long process) of transitioning it, by first slightly changing vibe-core's runApplication API, then updating all the examples and documentation, then phasing things out.

Before I get started on this rather large task, any chance I can get an ACK on the idea ?

@joseph-wakeling-frequenz

@Geod24 I'm with you on this, I don't like the static shared this approach at all.

One suggestion -- before approaching this:

The first step in doing so would be to remove any piece of documentation mentioning it.

... let's make sure that we do have well documented, front and centre, what one would consider the "right" way(s) to set up an application. That should probably be a precursor to all other changes.

I'd be happy to touch base some time on this topic to try to see if I can helpfully contribute.

@s-ludwig
Copy link
Member

I agree that we should go this route. shared static this was basically just a marketing feature in the early days to be conpetitive with scripting languages in terms of basic boilerplate. The single line that this saves is of course no real argument in any realistic scenario.

by first slightly changing vibe-core's runApplication API

What do you have in mind? Can this be an additive change?

The first step in doing so would be to remove any piece of documentation mentioning it.

I'd prefer to explicitly mark it as deprecated in the documentation, ideally mentioning the date of removal.

@Geod24
Copy link
Contributor Author

Geod24 commented Aug 25, 2021

What do you have in mind? Can this be an additive change?

I would have to re-visit the topic, since this issue has been opened a while ago. TBF after what you mentioned in the vibe-core issue, it might actually be much simpler than I expected.
In any case, I don't think I had any breaking change in mind.

I'd prefer to explicitly mark it as deprecated in the documentation, ideally mentioning the date of removal.

Okay, let me rephrase this: First step is to change all the examples to call runApplication and not ignore listen*'s return value.

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

No branches or pull requests

3 participants