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

.[] and .[]<- should show warning 3 when applied to String, not Bytes. #6706

Closed
vicuna opened this issue Dec 12, 2014 · 5 comments
Closed

.[] and .[]<- should show warning 3 when applied to String, not Bytes. #6706

vicuna opened this issue Dec 12, 2014 · 5 comments
Assignees
Labels

Comments

@vicuna
Copy link

vicuna commented Dec 12, 2014

Original bug ID: 6706
Reporter: @whitequark
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2016-12-07T10:37:10Z)
Resolution: fixed
Priority: normal
Severity: minor
Category: ~DO NOT USE (was: OCaml general)
Monitored by: @hcarty

Bug description

Currently it looks like this:

let x = "1";;

x.[0] <- '0';;

Warning 3: deprecated: String.set
Use Bytes.set instead.
Error: This expression has type string but an expression was expected of type
bytes

let xx = Bytes.create 1;;

val xx : bytes =

xx.[0] <- '0';;

Warning 3: deprecated: String.set
Use Bytes.set instead.

  • : unit = ()

So it does not work with String, and warns on Bytes. I think it should not warn when used as Bytes, given how -safe-string is the way forward.

@vicuna
Copy link
Author

vicuna commented Dec 13, 2014

Comment author: @whitequark

See #110.

@vicuna
Copy link
Author

vicuna commented Dec 13, 2014

Comment author: @gasche

When we devised -safe-string I discussed this with Damien, and our consensus at the time was that having the .[..] notation expect distinct types depending on how it would be used was confusing.

At first, Damien made .[] and .[] < -_ available for Bytes only, and I suggested that having .[] available for String was better usability-wise, as we expected most current uses of string to remain string (strings really being immutable in most situations). We agreed to do that even at the cost of making .[] <- _ unavailable in the -safe-string world.

I still don't think we need .[] <- _ for convenience when writing Bytes-heavy code. In my experience the explicit Bytes.set is fairly reasonable. Given that the "custom index operator" GPR #69 ( #69 ) would let user re-define their own indexing operators if they have a differing opinion, I would propose keeping the current state where only .[] is available, on strings rather than byte sequences.

@vicuna
Copy link
Author

vicuna commented Dec 13, 2014

Comment author: @gasche

I'm looking at the Github Pull Request (GPR) #69 again and it seems that, in this approach, the natural way to preserve backward-compatibility is to define ( .[]<- ) on Bytes -- and this would not give a deprecation warning unless we explicitly decide to tag it @@deprecated.

@vicuna
Copy link
Author

vicuna commented Dec 13, 2014

Comment author: @whitequark

I have no objections to #69 and this solution.

@vicuna
Copy link
Author

vicuna commented Dec 13, 2014

Comment author: @gasche

GPR 69 ( #69 ) was just merged, which should take care of this. Note that if we decide that deprecating ( .[]<- ) is important no matter what, we can still do it with [@@ocaml.deprecated].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants