Fix Issue 11938 - Add std.traits.CompatibleUnqual

[Poita]

CompatibleUnqual is similar to Unqual, except it only removes qualifiers when the unqualified type is compatible with the original type, i.e. is(T : CompatibleUnqual!T) is always true.

https://d.puremagic.com/issues/show_bug.cgi?id=11938

As mentioned in the bug, this is useful when writing range algorithms and you want to get a mutable version of a range (when possible).

[9rnsr]

I'm not sure CompatibleUnqual is really useful for range algorithm implementations. I recommend to add it with package until the usefulness is really proven.

[Poita]

I believe it is useful. Several uses of Unqual in std.algorithm should be CompatibleUnqual.

An example: this gives a compile error inside std.algorithm because Unqual is used in a constraint.

import std.algorithm, std.range;
void main()
{
    const(int[]) xs;
    const ys = inputRangeObject(xs);
    map!(a => a)(ys);
}

std/algorithm.d(411): Error: constructor test.main.MapResult!(__lambda1, const(InputRangeObject)
).MapResult.this (InputRangeObject input) is not callable using argument types (const(InputRange
Object))auto map(Range)(Range r) if (isInputRange!(Unqual!Range))

Problem is here:
auto map(Range)(Range r) if (isInputRange!(Unqual!Range))
Using CompatibleUnqual would make this fail in the constraint.

[9rnsr]

So why you don't fix the existing range algorithm issues with package CompatibleUnqual? You can prove the usefulness first.

[Poita]

@9rnsr I have given it package protection and fixed many uses throughout Phobos.

[JakobOvrum]

It will turn up in documentation so it should really be public and documented. You could prove its value with examples that fail with Unqual but work correctly with CompatibleUnqual (raises a different error), but of course it's difficult (impossible?) to verify those examples through unit tests when the difference is the error message.

[monarchdodra]

We could leave it as "documented but private": Eg, we document the behavior so users know what it means when they encounter it, yet still can't use it directly. We simply explain it is an "experimental" trait.

I too have some doubts about making it public, but documenting it won't hurt either. Best of both worlds I say :)

[JakobOvrum]

Documented private or package symbols are ignored by DDoc, so they won't show up in generated documentation. We can't expect users to have to dig in source code to figure out what a particular template constraint means.

[monarchdodra]

We can't expect users to have to dig in source code to figure out what a particular template constraint means.

Agreed.

Documented private or package symbols are ignored by DDoc, so they won't show up in generated documentation.

Groan. That's a bitch. Seems more like a limitation than a feature. How would one document things like "private virtual interface"?

[JakobOvrum]

CompatibleUnqual is just as useful for ranges defined in third-party libraries as it is for Phobos, so it seems strange to use it in Phobos constraints (i.e. public interface) but not have it public.

[Poita]

@9rnsr given the above discussion and the uses inside std.algorithm and std.range, are you happy for CompatibleUnqual to be public?

[JakobOvrum]

By the way, I'm pretty sure most of these algorithms don't actually need [Compatible]Unqual at all, and that Unqual was only used because it was before the head-qualifier stripping IFTI change? I think isInputRange!Range is correct for most of them?

edit:

Nevermind, I think CompatibleUnqual is correct for all higher-order range algorithms but not eager algorithms.

[9rnsr]

All of range algorithm fixes (replacement from Unqual to CompatibleUnqual) does not have corresponding unittests. So I'm not still sure these fixes are really correct.

I am really afraid about that adding more and more trait templates for certain situations. I really hate unnecessary bloating "D standard library".

[Poita]

I'd love to add unit tests, but the purpose of those changes is to avoid internal template errors by catching them in the constraint. I'm not aware of any way to test that.

I do understand your concern with library bloat, but I'm also concerned about all the blatantly incorrect uses of Unqual. The only reason it isn't a much bigger problem is because people don't use reference type ranges much -- they are very poorly supported. I'm not sure what the solution is here but the use of Unqual now is just wrong.

[ghost]

How would one document things like "private virtual interface"?

That's a good point. The NVI idiom (which doesn't currently work) comes to mind.

[monarchdodra]

I'm not sure about having all these functions accepting const ranges, since the const can be stripped at the call site, using CompatibleUnqual of course :), reducing template bloat to boot. That said, this change is non-breaking, and more correct then before, so I wouldn't turn it down because of that.

I mostly had this in mind for RoR kind of algorithms, where stripping the "internal" const at call site is much harder: The original bug report of:

immutable words string[] ["hello", "there"];
words.join(" ").writeln(); //"Should" work with IFTI, but fails on "second level" immutable

Anywhoo, I found another use case that CompatibleUnqual solves: Repeat of const elements: Because the range caries the element type (which is const), the range itself becomes non-assignable, and non-"copy-saveable" (see: https://d.puremagic.com/issues/show_bug.cgi?id=12007)

By having Repeat!T store a CompatibleUnqual!T, we'd be able to better support having "range of const elements".

Or more generally any type that models containing a const T.

[monarchdodra]

There is still the DDoc issue (which I can live with), but otherwise, for me, this is a welcome change and seems good to go. Any one have anything else to add?

BTW, couldn't the ddoc issue be solved with a version(STD_DDoc) where it's public? Or maybe that could create subtle bugs for doc/non-doc builds. Better to fix DDoc than do something dangerous I think.

[Poita]

I don't want this pulled if @9rnsr is not happy with it. It's not a pressing issue, and maybe there is a language change that could make it unnecessary.

[andralex]

I'd call it ImplicitUnqual because it does thing aking to C++'s implicit_cast. The meaning seems to be "remove top-level const or immutable" or HeadUnqual. I'm not sure about the usefulness.

[quickfur]

ping
Are we going ahead with this?

[mihails-strasuns]

I wonder if we should instead add a template flag for Unqual which defines exact un-qualification strategy (with "strip all" being the default for compatibility). It allows for some more expressive power than many templates with similar names.

[Poita]

I'm not sure what to do here. Andrei and Kenji are not convinced this is good, and I wouldn't want something going in when key people are not on board.

Some things are clear to me:

• We have many uses of Unqual in Phobos that are just plain wrong. In fact, almost every use of Unqual in Phobos is incorrect (see the file changes in this diff). If you are unconvinced, try use a const class-type range with most Phobos functions.
• You generally want head const to be stripped with IFTI, as it is in C++. D does this for arrays and pointers, but nothing else. You can't strip the head const with structs with indirections.

Ideally I'd like this to be solved in the language, then this diff (and all the brokenness in Phobos) goes away. Head const should always be stripped, but to do that we'll need to find a way to copy const(T) to T, which is currently impossible.

[mihails-strasuns]

Ideally I'd like this to be solved in the language

So do I but this issue has been discussed for ages with no good solution found so far. It can take few ages longer :( I agree though that there is no point in spending more time on this if @andralex and @9rnsr both do not like it - probably this pushes the change into the DIP domain.

[mihails-strasuns]

I will close this until it is either supported by more fundamental justification (thinking DIP) or @andralex naturally changes his mind.