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

Containers: implement also StringView and char concatenation #134

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mosra
Copy link
Owner

@mosra mosra commented May 26, 2022

Because why do the extra work with getting a size of the view if we don't need to.

This however results in an ambiguity with ArrayView<[const ]char> + int, i.e., when both sides of the expression need an implicit conversion -- then a candidate is either the builtin operator+(const char*, std::ptrdiff_t) , or operator+(StringView, char). Possible solutions:

  • Disable implicit ArrayView to pointer conversion, forcing people to do view.data() + 3 instead of view + 3? Needs to be done in a backwards-compatible way however, as a lot of code relies on this... so it's not an immediate solution.
    • Could fix also many other annoyances (like nasty ambiguity when creating a StringView from an ArrayView<char>, which also picks the const char* overload)
  • Disable implicit conversion between ArrayView and StringView? Would make a lot of new APIs nasty (such as Path::write() no longer accepting string views for data), OTOH could also prevent errors when write() accidentally gets data first and filename last. A lot of code also relies on this already, meaning it needs backwards compat and thus this is also not an immediate solution.
  • (Ew.) Move the operator+() to String.h so we can have it templated and add a nasty SFINAE that picks only directly a StringView. Or a String? Or .... ugh. This would also mean trying to call operator+() when only StringView.h is included would fail with "no such operation possible", which isn't really nice to users.

Because why do the extra work with getting a size of the view if we
don't need to.

TODO: ambiguous with `ArrayView + int`, how to fix?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

1 participant