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

str1 + str2 + str2 should be optimized #276

Open
medvednikov opened this issue Jun 19, 2019 · 12 comments
Open

str1 + str2 + str2 should be optimized #276

medvednikov opened this issue Jun 19, 2019 · 12 comments
Labels
Type: Optimization This issue/PR is related to optimizations that could be applied to V.

Comments

@medvednikov
Copy link
Member

Concatenating multiple strings should always result in only one allocation.

@medvednikov medvednikov added the Type: Optimization This issue/PR is related to optimizations that could be applied to V. label Jun 19, 2019
@joe-conigliaro
Copy link
Member

joe-conigliaro commented Jun 20, 2019

Have you thought about python style string formatting "hello {} {}".format(firstName, lastName) or something similar?
Edit: I guess we already have concatenation (+) and interpolation with variables ($var) so don't need the complexity

Also for joining and splitting arrays ",".join(array) or array.split(",") ?

@medvednikov
Copy link
Member Author

Edit: I guess we already have concatenation (+) and interpolation with variables ($var) so don't need the complexity

You've answered your question correctly :)

Also for joining and splitting arrays ",".join(array) or array.split(",") ?

str_array.join(','), but no array splits.

@gslicer
Copy link

gslicer commented Sep 24, 2019

Is it now
str1 + str2 + str3
or
str1 << str2 << str3
?

@Delta456
Copy link
Member

@gslicer the first one

@gslicer
Copy link

gslicer commented Sep 24, 2019

@Delta456

@gslicer the first one

That seems again inconsistent with the Array notation where it is stated (according to docs):
"<< is an operator that appends a value to the end of the array. It can also append an entire array. "

So I would expect string (which is a [byte|rune]-Array to support the same notation - why not let + be pure mathematical?

When there was something I liked in C++ it was the "<<" operator on iostreams (which were basically byte-streams or var-length strings ;)

@medvednikov
Copy link
Member Author

string is not an array.

@gslicer
Copy link

gslicer commented Sep 25, 2019

I'm sorry @medvednikov but in current implementation string looks like an array to me (and you even called the variable representing a string "arr" which resambles Array):

module strings

pub fn repeat(c byte, n int) string {
	if n <= 0 {
		return ''
	}
	//mut arr := malloc(n + 1)
	mut arr := [byte(0)].repeat(n + 1)
	for i := 0; i < n; i++ {
		arr[i] = c
	}
	arr[n] = `\0`
	return string(arr, n)
}

My point is, + could be used only for arithmetic operations, and << for concatenation (of any kind) - this would be more streamlined with the "simple" and "do similar stuff with similar syntax" approach, don't you think?

@medvednikov
Copy link
Member Author

No, it's not an array. String is an immutable struct with a pointer to string data, len, and capacity integers.

In this function arr refers to []byte from which a string is generated via string().

@medvednikov
Copy link
Member Author

medvednikov commented Sep 25, 2019

By the way thanks for referring to this function, it needs to be updated :)

@adler-amorette
Copy link

adler-amorette commented Feb 22, 2022

Things like that may be nice, but not only it's additional complexity in the compiler, but also it's not even obvious that there are such optimizations, so it's one more random thing to learn.
I could say that maybe it's worth making it more general, like make operators variadic. But that's just complicating something that is already simple.
I would say that ''.join(s1,s2,s3) is fine, and even ''.join(s1,s2) is fine. Like if only in C++ there was no + for std::string, so much of code would be automatically optimized, you would just call concat(or whatever it would be called). So I think it's fine to remove operator overloading all together, because it inevitably leads to slow code. Let's not repeat the same mistakes.

@esquerbatua
Copy link
Contributor

Related: #22188

@JalonSolov
Copy link
Contributor

Things like this should be simple in the new compiler. The IL will make optimizations much simpler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Optimization This issue/PR is related to optimizations that could be applied to V.
Projects
None yet
Development

No branches or pull requests

7 participants