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

textfilter utilities don't properly use output ranges #2541

Closed
schveiguy opened this issue Mar 18, 2021 · 3 comments
Closed

textfilter utilities don't properly use output ranges #2541

schveiguy opened this issue Mar 18, 2021 · 3 comments

Comments

@schveiguy
Copy link
Contributor

An output range is properly defined as working like:

import std.range : put;
put(somerange, somedata);

But vibe.textfilter.urlencode calls it like:

somerange.put(somedata);

This eliminates ranges such as delegates that accept a const(char)[]. The code should use std.range.put exclusively to support all output ranges.

@s-ludwig
Copy link
Member

Isn't that just a matter of a missing an import of std.range.primitives then, or are there cases where relying on UFCS would result in the wrong function to be called (due to all the extra logic in the global one)?

@schveiguy
Copy link
Contributor Author

No, unfortunately. This is one of the failures of Phobos. Because the hook to put is called put, it's very easy to use the member instead of the std.range function as UFCS, which means using r.put is going to limit you only to what the type supports. It will work for some ranges, but not others (as evidenced here).

It's also possible to have an output range of dchar that just has a put(dchar) member, which means the textfilter modules won't compile with that output range, even though isOutputRange!(R, dchar) is true.

The annoying answer is, you can't use put via UFCS in implementations, you have to call it as a standalone function to make sure it's properly used.

I can make a PR, just wanted to leave this issue report here so it's not forgotten.

@schveiguy
Copy link
Contributor Author

PR made, see #2542

s-ludwig added a commit that referenced this issue Mar 19, 2021
Fixes #2541 - use std.range.put properly for output ranges.
gedaiu pushed a commit to gedaiu/vibe.d that referenced this issue Apr 2, 2021
gedaiu pushed a commit to gedaiu/vibe.d that referenced this issue Apr 3, 2021
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

2 participants