Fix Issue 21456 - std.format does not accept enum member with string base type as template parameter

[berni44]

Don't know if it wouldn't be better to have to return type just being auto...

[dlang-bot]

Thanks for your pull request and interest in making D better, @berni44! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	21456	normal	std.format does not accept enum member with string base type as template parameter

Testing this PR locally

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

dub run digger -- build "master + phobos#8029"

[thewilsonator]

Don't know if it wouldn't be better to have to return type just being auto

if that works then yes please

[RazvanN7]

Why are these imports made public?

[berni44]

Sorry, I needed that for stating the return type explicitly and forgot to undo this, when using auto.

[andralex]

This is distinctly the wrong kind of generality.

I am very sorry we missed the initial support for enum types derived from strings. No we don't need to support them. Whoever has those enums may as well call representation and call it a day.

This pull should be undone and all support for enum strings should be deprecated.

[berni44]

This is distinctly the wrong kind of generality.

I am very sorry we missed the initial support for enum types derived from strings. No we don't need to support them. Whoever has those enums may as well call representation and call it a day.

This pull should be undone and all support for enum strings should be deprecated.

For me it is OK to revert this PR - I just tried to care about a bug and marking this bug as invalid would achieve my goals too.

Anyway, I would like to understand what is wrong with enum types derived from strings? If I would have known this, I would have probably marked the issue immediately as invalid and gave a reason there. Maybe you can point me to a place, where this is explained? (Or explain it yourself in brief?)

[nordlow]

This is distinctly the wrong kind of generality.

I am very sorry we missed the initial support for enum types derived from strings. No we don't need to support them. Whoever has those enums may as well call representation and call it a day.

This pull should be undone and all support for enum strings should be deprecated.

A great motive for ditching enum strings is that it avoids duplicated template instances.

For instance

void format(alias fmt)() { pragma(msg, fmt); }

unittest {
    format!("%s")();
    enum Fmt : string { s = "%s" }
    format!(Fmt.s)();
}

prints

%s
%s

meaning format is instantiated twice whic is completely unnecessary. This creates template bloat that slows down the compiler.

Does this hold for enum's based on other types aswell such as aggregates, @andralex ?

[Geod24]

I am wondering, why is fmt an alias and not a simple string ? The only reason I can think of is to support wstring and dstring, but I wouldn't find that compelling, as fmt has to be CT-known, and the main reasons to have non UTF8 string types is either interacting with other APIs or micro optimizing for memory usage.

Having fmt be a string would have avoided the need for special support here. From a quick look, it also seems that reverting this will prevent types with alias this from working. Not that I think it's a good idea to support it (or enum format string), but if we do, it shouldn't require special code to do so.

[berni44]

I am wondering, why is fmt an alias and not a simple string?

Don't know. It was introduced in #5288 and not changed since. At least no unittest complains, when I replace it with string fmt and remove the constraint.

[Geod24]

CC @ntrel

[ntrel]

why is fmt an alias and not a simple string ?

No real reason. I think I just used alias whilst working on the change as it's more flexible, then never thought to tighten it to string before submitting.

[schveiguy]

Having fmt be a string would have avoided the need for special support here.

This is probably the right answer. It should work with anything that currently works, except wstring and dstring formats. For those, you can probably use a converter, or just generate string in the first place.

A good first step might be reverting this, and seeing if replacing alias with string breaks anything.

[schveiguy]

Note that we still would need to deprecate if we did move to accepting a string directly.

[nordlow]

There are more format specificers being alias that should be string in the helper functions for format. Grepping for alias fmt gives 9 hits in Phobos. They are all except one using if (isSomeString!(typeof(fmt))).