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

Pervasives.string_of_bool doesn't create new string #5493

Closed
vicuna opened this issue Jan 26, 2012 · 5 comments
Closed

Pervasives.string_of_bool doesn't create new string #5493

vicuna opened this issue Jan 26, 2012 · 5 comments
Assignees
Labels

Comments

@vicuna
Copy link

vicuna commented Jan 26, 2012

Original bug ID: 5493
Reporter: @johnwhitington
Assigned to: @lefessan
Status: closed (set by @xavierleroy on 2013-08-31T10:46:19Z)
Resolution: not fixable
Priority: normal
Severity: minor
Category: ~DO NOT USE (was: OCaml general)

Bug description

feast:trunk john$ ocaml
Objective Caml version 3.12.1

let x = string_of_bool false;;

val x : string = "false"

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

  • : unit = ()

x;;

  • : string = "falxe"

string_of_bool false;;

  • : string = "falxe"

Is this the intended behaviour? It seems to break abstraction somewhat.

Solution would be to use string_blit and string_create from pervasives.ml to build a new string each time. I would be happy to write the patch if required.

@vicuna
Copy link
Author

vicuna commented Jan 26, 2012

Comment author: @protz

Other funny example:

let f () = "toto";;

val f : unit -> string =

let v = f () in v.[1] <- 'a';;

  • : unit = ()

f ();;

  • : string = "tato"

There was a discussion about that on the caml-list, you may be able to find it by crawling the archives of the google group. The tl;dr is that strings are optimized, and shared; therefore, you shouldn't think of them as something mutable.

I'm inclined to close this as "not a problem" but I'll wait for others to weigh in...

@vicuna
Copy link
Author

vicuna commented Jan 26, 2012

Comment author: @johnwhitington

I guess that makes sense. I just found it confusing on a fundamental API boundary. Perhaps it deserves mention in the documentation?

@vicuna
Copy link
Author

vicuna commented Jan 26, 2012

Comment author: @protz

Well I guess a small disclaimer somewhere near the documentation of the [String] module would be worthwhile. I'll check with others see if we should recommend some sort of rope library for efficient sharing...

@vicuna
Copy link
Author

vicuna commented Jan 26, 2012

Comment author: @lefessan

I changed the documentation of Pervasives.string_of_bool to state that the returned value may be shared and should not be modified directly.

A lot of functions that return strings should actually be documented in the same way, for example when some of them may return the value received as argument when no modification is required.

Users should use String.copy to guarantee that they are no modifying a shared string.

@vicuna
Copy link
Author

vicuna commented Jan 30, 2012

Comment author: @protz

r12102 also adds an extra comment in string.mli about that "feature"

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