-
Notifications
You must be signed in to change notification settings - Fork 14
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
"Should be a compile time constant" warning when using nameof #9
Comments
Well, after some research, I think that it's not a mistake. The compilation of this code fails const string a = $"Hello {nameof(String)}"; But this code is valid const string a = "Hello " + nameof(String); You may say that this irrelevant in the context of Serilog templates. Because both interpolation and concatenation produce an immutable string, but that's not completely true either. Let's look at IL for the code Console.WriteLine($"Hello {nameof(String)}"); On .NET Core 3.1+ it creates a constant IL_0000: ldstr "Hello String"
IL_0005: call System.Console.WriteLine
IL_000A: ret But on old full .NET Framework IL_0000: ldstr "Hello {0}"
IL_0005: ldstr "String"
IL_000A: call System.String.Format
IL_000F: call System.Console.WriteLine
IL_0014: ret And this is a performance issue
So I'm not sure if we want to treat the interpolation as a constant. Please let me know what you think. P.S. |
Interesting, didn't consider what is happening under the hood. Ultimately, I don't think it's a big deal - if this is a potential performance issue then the warning is warranted. The only improvement I can think of would be to split off the warning into a different analyzer (something like "nameof() incurs a performance penalty"), maybe even detecting if it's running on Framework or .NET Core 3.1+ if that's possible, so it only gets displayed if it doesn't get compiled into an actual constant. |
Hi @lennartb-,
const string a = "String";
Console.WriteLine($"Hello {a}"); IL_0000: ldstr "Hello {0}"
IL_0005: ldstr "String"
IL_000A: call System.String.Format
IL_000F: call System.Console.WriteLine As you can see using a constant as a placeholder produces the same result.
I believe this is possible. But relying on pretty low-level compiler optimization is dangerous. Log.Information("Hello " + nameof(String)); or Serilog way Log.Information("Hello {ClassName}", nameof(String)); You'll get a |
Closed due to inactivity, feel free to re-open the issue. |
@olsh Looks like C# 10 has constant interpolated strings, giving this issue some fresh wind: |
Example:
Log.Information($"Hello {nameof(MyClass)}");
causes a "Message template should be a compile time constant" warning, butnameof
expressions are evaluated at compile time.The text was updated successfully, but these errors were encountered: