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

Discussion: lightweight string interpolation #369

Closed
alrz opened this issue Mar 28, 2017 · 15 comments
Closed

Discussion: lightweight string interpolation #369

alrz opened this issue Mar 28, 2017 · 15 comments

Comments

@alrz
Copy link
Member

alrz commented Mar 28, 2017

It would translate to simple string concatenations,

var a = $$"{key}: {value}";
// equivalent to
var a = key + ": " + value;

No "format specifier" may be used in this context.

If all interpolations are constants, it can be used as a constant:

[Attribute($$"{constant_key}: {constant_value}")]

Note: we use the "result of an invariant ToString" for literals to retain constness and culture-invariance.

Alternatively, we could just allow the existing $ interpolations to produce a constant, if the said constraint holds. As @HaloFour pointed out in #384 (comment) that would cause different results from a regular vs. constant string interpolation literal which can be confusing.

EDIT: Something like $"{value}"i might be easier to understand, and won't look too stupid in verbatim mode: $@"{value}"i vs. $$@"{value}".

@svick
Copy link
Contributor

svick commented Mar 28, 2017

There already is a way to get invariant string interpolation:

using static System.FormattableString;var a = Invariant($"{key}: {value}");

Though it's not a constant, so it can't be used in attributes. (And it also allocates even more than normal string interpolation.)

And the issue with considering normal string interpolation to be constant is that it is not. It depends on the current culture and for example $"{3.14}" will return different values for different current cultures, even though 3.14 is a constant. So I don't see how would the alternative version of this proposal work.

@alrz
Copy link
Member Author

alrz commented Mar 28, 2017

@svick

And the issue with considering normal string interpolation to be constant is that it is not. It depends on the current culture and for example $"{3.14}" will return different values for different current cultures, even though 3.14 is a constant. So I don't see how would the alternative version of this proposal work.

That issue applies to the $$ syntax as well. We should use the result of an invariant ToString for literals.

@iam3yal
Copy link
Contributor

iam3yal commented Mar 29, 2017

I'd really love to have something like this.

@related #177 AKA "We need more control over string interpolation". :)

@alrz
Copy link
Member Author

alrz commented Mar 29, 2017

@eyalsk

As @svick pointed out, if you just want invariant culture you could pass it to FormattableString.Invariant. This issue mainly aims for a lightweight (and possibly constant) string interpolation literals.

@alrz alrz changed the title Discussion: culture-independant string interpolation Discussion: lightweight string interpolation Mar 29, 2017
@iam3yal
Copy link
Contributor

iam3yal commented Mar 29, 2017

@alrz So you didn't read the proposal I linked. :)

I'm fully aware that these are two separate proposals, I just linked because both provide options to string interpolation or more control over it.

@alrz
Copy link
Member Author

alrz commented Mar 29, 2017

@eyalsk

I did, that's why I mentioned the Invariant method. I changed the title to distinguish the two proposals.

@MgSam
Copy link

MgSam commented Mar 29, 2017

Do you have a measurable perf problem caused by string interpolation? If you do, it seems like stepping down and using a StringBuilder would be more appropriate in such a case. Interpolation causing perf issues seems like a pretty rare case to warrant adding a whole new type of string syntax.

@alrz
Copy link
Member Author

alrz commented Mar 29, 2017

@MgSam

Just like linq which is a no-go for perf critical code, string interpolation is recommended against in such use cases. Not to mention, it cannot be used as a constant, which is really inconvenient when you need it.

@jnm2
Copy link
Contributor

jnm2 commented Mar 30, 2017

@eyalsk

So you didn't read the proposal I linked. :)

Ahem... ;-)

@ig-sinicyn
Copy link

@MgSam

it seems like stepping down and using a StringBuilder would be more appropriate

Well, almost all non-trivial scenarios that use string interpolation incur unexpected (by most developers) performace hit. As example,

log.Write($"Some diagnostic string with {some} {args} ...")

this single line gave us +2.5 Gb total allocations per hour (string formatting was unneeded as logging was disabled in config).

Sadly, it's by design and it does not look like a thing to be fixed.

@MgSam
Copy link

MgSam commented Mar 30, 2017

What the use cases when you have string constants that require interpolation?

@jnm2
Copy link
Contributor

jnm2 commented Mar 30, 2017

@MgSam
[DebuggerDisplay($"{nameof(X)}, {nameof(Y)}")]
[Description($"Adds the ability to foo ({nameof(DoFoo)}) [...]")] for parity with XML doc comment using <see cref="Foo"/>

@alrz
Copy link
Member Author

alrz commented Mar 30, 2017

@MgSam The first time I've encountered this limition in attributes, #384 provided a concrete example.

@Pzixel
Copy link

Pzixel commented Mar 30, 2017

@ig-sinicyn however, libs like NLog forbid such usages and inspirit to use their API's ( https://github.com/NLog/NLog/wiki/Tutorial#writing-log-messages ). Even if this feature gets implemented you still have an allocation for string concatenation result.

However, I think it should be implemented

@alrz
Copy link
Member Author

alrz commented Jun 1, 2018

Th optimization went in via dotnet/roslyn#26612

Closing in favor of #384 for constant string interpolations.

@alrz alrz closed this as completed Jun 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants