deprecate hex string literals

[ghost]

The library version was merged 2 years ago so it can be qualified as bug-free, allowing to remove the built-in version.

[dlang-bot]

Thanks for your pull request, @bbasile! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

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.

[wilzbach]

If this gets accepted, we shouldn't forget to update the deprecation page

[wilzbach]

Nit: You could link to http://dlang.org/spec/lex.html#HexString

[ghost]

Actually i see a slight problem caused by the removal of built-in hex strings: people who compile betterC D code will be, in a way, discriminated.

[WalterBright]

These should still be tested, even while deprecated, as they are still supported.

[JinShil]

Actually i see a slight problem caused by the removal of built-in hex strings: people who compile betterC D code will be, in a way, discriminated.

Those who compile with -betterC are already discriminated against in much more serious ways. Don't worry about it. It's a tradeoff that they voluntarily opt into.

[wilzbach]

@WalterBright - I think this needs your approval. Could you please quickly confirm that you are ok with deprecating hex strings (and I guess in the next step octal strings) so that @bbasile can move on here and get this through the auto-tester?

[ghost]

I've detected a problem while removing the uses in phobos:

void main()
{
    import std.conv: hexString;
    enum a1 = cast(ubyte[4]) x"40 41 42 43"; // compiles
    enum a2 = cast(ubyte[4]) hexString!"40 41 42 43"; // fails
}

both versions are not perfectly interchangeable. The library version doesn't allow cast to static arrays so there's a risk of of breakage, even if hex strings are finally rarely used, but well deprecation phase is there for this i suppose.

[MartinNowak]

enum ubyte[4] a2 = hexString!"40 41 42 43"; // works

Don't ask me why though :/.

[ghost]

I'd like to put the focus on this PR: dlang/phobos#5546.
You can see the consequences here. I think that it was not a good idea. Since the compiler and the standard library are decoupled the consequence is that the tests are not run at all when one makes a PR that proposes a deprecation.

You can also see that this affects a simple PR from a very casual committer. Is that just a random fact ? I don't think so.

[wilzbach]

You can see the consequences here. I think that it was not a good idea. Since the compiler and the standard library are decoupled

They have always been coupled (de facto). After all, that's why the auto-tester tests all three repos.

the consequence is that the tests are not run at all when one makes a PR that proposes a deprecation.

I am not sure I get your point, the deprecation messages are shown:

https://auto-tester.puremagic.com/show-run.ghtml?projectid=1&runid=2651217&isPull=true

Are you complaining about the fact that CircleCi fails and as a result doesn't report coverage? Or that most CIs are red?

You can also see that this affects a simple PR from a very casual committer.

It doesn't. This PR is blocked on @WalterBright's approval for changing the D language, when/if there's the approval, we can go ahead and merge your Phobos PR.

[ghost]

No. The test are failing since yesterday because you've decided (I mean you and the other member who has approved) that the deprecations are now errors instead of warnings in phobos. Cant you see the problem ? If in the future something has to be deprecated the same will happen again.

[CyberShadow]

I am not sure I get your point,

@bbasile is complaining that now that Phobos is failing the build, the autotester never gets around to running the DMD test suite.

TBH I don't see much of a difference - you do want to have all uses of deprecated features removed from Phobos before the compiler change is merged, as it not only ensures there are no embarrassments about us ourselves still using features we don't want others to use, but also ensures that we have suitable replacements that are at least as good for Phobos. Whether to deprecate something also depends more on whether the idea is sound (for which being able to adequately fix the deprecation instances in Phobos makes a good litmus test), and secondly on the quality of the implementation (which the DMD test suite checks).

Furthermore, you wrote about "platform specific failure missed locally", but deprecations by their nature are going to be a front end change, so there is not going to be much platform-dependent about it.

I guess it would be possible in theory to check for deprecations in the Phobos tests instead of build, but I don't think the make file complications are worth it. Get your Phobos PR merged (dlang/phobos#5507), and then you will see DMD test results again.

[ghost]

The difference is so obvious. From everything green yesterday to everything red today. Your new policy doesn't allow to test and integrate correctly new deprecations at the language level.

[CyberShadow]

From everything green yesterday to everything red today.

As it should be. We should not be using features that we have deprecated in any components of D.

Another point I forgot to mention above in that occasionally through negligence, we had shipped a version of D which caused deprecation warnings in Phobos when it is used as a library, i.e. someone imports a non-deprecated module and uses a non-deprecated symbol, and they get deprecation errors somewhere inside Phobos source code, making their program unbuildable with -de. This is a situation we definitely want to avoid.

That this PR was showing as green in the autotester was a problem, which has now been fixed. The correct process is to first fix uses of deprecated symbols in Phobos/Druntime/etc., then have the deprecation merged.

[ghost]

So i let the initiative to someone who'll be stasified by the way things are working here