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

string interpolation simple cases are not optimized (such as: string s1 = $"hello") #76

Closed
dzeitlin opened this issue Jan 25, 2015 · 9 comments
Assignees
Labels
Area-Compilers Bug Language-C# Resolution-Not Reproducible The described behavior could not be reproduced by developers Verified
Milestone

Comments

@dzeitlin
Copy link

string s1 = $"hello"; emits string s1= string.Format("hello", new object[0]);
instead of simple: s1 = "hello";

also consider to optimize cases such as:
$"hello" + $" " + $"world" to constant expression evaluation (or at least one string.Format() call, not 3 in this case)

@dzeitlin
Copy link
Author

Thanks you right, I chcked this in oficial CTP5 release.
But still even in master branch:
Code like: $"hello {null} {objNull}" is not optimized as a classic string, see:
http://tryroslyn.azurewebsites.net/#b:master/K4Zwlgdg5gBAygTxAFwKYFsDcAobBjAGwEMQQYBhGAb2xjpgAcAnMANyLRjwHsIUZuAIwBWqPMgEiAcsAIEYAXhgRZBHPRi16KFtHgBGRQD4YAEgBEAC1Rzu1FXIC+1IcJlPz67cl2w4AJmMYKxsCbnMYAGplVSjJN1UcRyA

@paulomorgado
Copy link

What if the current culture has a formater that does something with nulls?

@stephentoub
Copy link
Member

And how common is it in performance-critical situations to be doing string formatting with constant null values? Even if it could be optimized, seems extremely corner-case, or am I missing something?

@mikedn
Copy link

mikedn commented Jan 26, 2015

Is there a valid reason to ever write something like $"hello"? If not then maybe this should be a compiler warning.

@s-aida
Copy link

s-aida commented Jan 26, 2015

Perhaps when an interpolated variable refers to null?

var text = $"hello{world}";

@AdamSpeight2008
Copy link
Contributor

Why was String.Format chosen to be the target method? Rather than String.Join?

Dim text = $"hello{world}"

into

Dim text = String.Join(String.Empty,{ "hello",world } )

If any of the arg holes have alignment or format args then make the target String.Format.

@paulomorgado
Copy link

Like I said before on this issue, my current culture might have custom formatting for that.

The predictable way is to always be a call string.Format. There's already a pretty good syntax for string.Concat: +. Which is a lot better than string.Join because it doesn't involve an array invocation and iteration just to have an empty string as a separator.

@gafter
Copy link
Member

gafter commented Feb 3, 2015

I cannot reproduce this "bug". I see the code in the compiler to perform this optimization and your test appears to show it working properly:

http://tryroslyn.azurewebsites.net/#b:master/K4Zwlgdg5gBAygTxAFwKYFsDcAobBjAGwEMQQYBhGAb2xjpgAcAnMANyLRjwHsIUZuAIwBWqPMgEiAcsAIEYAXhgRZBHPRi16KFtHgBGRQD4YAEgBEAC1Rzu5nAF8gAA

@gafter gafter closed this as completed Feb 3, 2015
@gafter gafter added 4 - In Review A fix for the issue is submitted for review. Resolution-Not Reproducible The described behavior could not be reproduced by developers and removed 1 - Planning labels Feb 3, 2015
@theoy theoy removed the 4 - In Review A fix for the issue is submitted for review. label Feb 4, 2015
@gafter gafter assigned theoy and unassigned gafter Feb 10, 2015
@gafter gafter added the Verified label Mar 9, 2015
@theoy theoy assigned jaredpar and unassigned theoy May 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Language-C# Resolution-Not Reproducible The described behavior could not be reproduced by developers Verified
Projects
None yet
Development

No branches or pull requests

10 participants