Fix Issue 18325 - Don't display TLS variables when building DMD

[wilzbach]

The first -vtls flag was added in 7a087dc, but no one seems to know anymore why it was added.

AFAICT was -vtls is a legacy flag from the long gone migration to shared.

Alternative PR: dlang/druntime#2138

[dlang-bot]

Thanks for your pull request, @wilzbach!

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.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#8018"

[jacob-carlborg]

As far as I know we don’t use TLS variables in DMD. Doesn’t this flag help showing when a TLS variable is accidentally added?

[wilzbach]

As far as I know we don’t use TLS variables in DMD

TLS variables are used in druntime, hence the warning and as the warning has been there for quite a while, it doesn't seem to break anything...

Doesn’t this flag help showing when a TLS variable is accidentally added?

Yes, but I don't see any reason why TLS variables shouldn't be used...

[ibuclaw]

It was used primarily to make sure that nothing was silently changed in the C++ - > D conversion.

It is subsequently there as a reminder to not accidentally break the extern interface if you intend on exposing a new static var to other languages.

There is a possible future™ design where parsing/semantic of all modules are done asynchronously. But I think the only person interested in that is Walter.

[schveiguy]

It is subsequently there as a reminder to not accidentally break the extern interface if you intend on exposing a new static var to other languages.

Except who is watching this? If it's not tested in CI, then it's useless.

I vote to remove it.

[schveiguy]

Alternatively, you can leave it in for certain builds, but it needs to be flagged as an error, not just "information" that only helps if you read the build logs fully.

And the messages from druntime need to be ignored in this case.

[marler8997]

Except who is watching this? If it's not tested in CI, then it's useless.

I watch it since I see the resulting messages every time I build the compiler.

[schveiguy]

@marler8997 what I mean is that nobody is reading meticulously the build logs from the auto tester, so if it's not being caught there, then if someone adds something that triggers a message, nobody notices. Sure, you will notice after it's merged and potentially released. But that's useless IMO.

[ibuclaw]

It is subsequently there as a reminder to not accidentally break the extern interface if you intend on exposing a new static var to other languages.

Let me rephrase. Thread local variables should be kept in check as they cannot be reliably accessed outside of D code. Think platforms where dmd rolls their own tls storage.

Removing this outright is not the way to go, you could add it to DFLAGS perhaps though? Maybe make it part of the DEBUG builds and hope that you never introduce silent corruption for gdc and ldc which will annoy us royally for the hours of debugging wasted.

[schveiguy]

@ibuclaw I have no problem with that idea, but clearly, TLS is used inside the front end (as druntime is using it), and is currently passing all CI tests (therefore, there is no automated test for checking for TLS variables). The result is: the flag isn't doing anything productive.

We should either remove the druntime usage (perhaps for a DMD-only build) and enforce that TLS usage is gone from the front end, or figure out a way that front end can reliably use TLS.

The current situation is like fixing a bug and not adding a test for it.

[wilzbach]

So what's the status on this? Maybe we can get an opinion from @andralex / @WalterBright?

We should either remove the druntime usage (perhaps for a DMD-only build) and enforce that TLS usage is gone from the front end, or figure out a way that front end can reliably use TLS.

I agree. That current status is a weird in-between state in which we have neither of both and it also scares/confuses newcomers with this rather unrelated build error.

[wilzbach]

Closing as this isn't very important to me.