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

Add improved interpolated strings spec #4486

Merged
merged 13 commits into from
Mar 25, 2021

Conversation

333fred
Copy link
Member

@333fred 333fred commented Mar 1, 2021

This is the spec we reviewed in LDM today (03/01/2021).

333fred and others added 8 commits February 18, 2021 17:49
* Some grammar cleanup
* Add additonal motivation around Span
* Look for a constructor on the type, rather than a static method, when looking for an interpolated_string_builder_conversion
* Clarify that we can look either on instance receivers or containing types for GetInterpolatedString methods.
* Slightly adjust the better conversion rules to ensure that constants still bind to string overloads, and add a detailed example explaining the resulting rules.
* Example cleanup
* Add discussion and examples of other discussed use cases.
* Flip the script on the receiver. Rather than calling a method on the receiver, we pass the receiver to a static method. This unifies the way of obtaining a builder.
* Always obtain the builder from a static method. This allows delegation to constructors, or more advanced methods if the implementor chooses.
* Move the builder to an out parameter of the static method. This allows overloading of the GetInterpolatedStringBuilder method. Updated the logging example to show how this would help.
* Added open questions as appropriate.
@danmoseley
Copy link
Member

cc @JeremyKuhne

@ufcpp
Copy link

ufcpp commented Mar 2, 2021

Can this improved interpolated strings be used in async method?

async Task m1()
{
    await Task.Delay(1);

    {
        // Ref structs can not be used in async methods even if it does not spill out of await expressions.
        Span<byte> span = stackalloc byte[1]; // error.
        span[0] = 3;

        // So what about the builder?
        InterpolatedStringBuilder.GetInterpolatedStringBuilder(47, 1, temp, out var builder); // error?
    }

    await Task.Delay(1);
}

@333fred
Copy link
Member Author

333fred commented Mar 2, 2021

It's a good question @ufcpp, and we'll need to figure out how that will work.

@stephentoub
Copy link
Member

For reference, here's approximately what I expect the runtime support for this would look like, focusing on being able to create strings and formatting into existing spans:
stephentoub/runtime@df859de

@stephentoub
Copy link
Member

stephentoub commented Mar 2, 2021

It's a good question @ufcpp, and we'll need to figure out how that will work.

I would expect/hope it would "just work". The limitation on not using ref structs in async methods isn't a runtime limitation but a limitation the compiler currently enforces on code the dev writes; I don't know if it also enforces it on IL it generates, but it shouldn't, and if it does, we can just stop doing that 😄 If the concern is using await in the holes, then if await is used it can bind to the old mechanism if targeting string or fail to bind if targeting a custom ref struct builder type. My $.02.

we pass the receiver, and we don't in the general case?), and what we do if parameters _after_ the string parameter are annotated, but it seems like a potential option.

Some compromise is likely needed here, but either direction has complications. Some scenarios that would be affected by this is the `Utf8Formatter` below, or existing api patterns that have
an `IFormatProvider` as the first argument.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding this section in response to my pestering you about it. ;-)

I would like us to be able to add:

String.Format([InterpolatedStringArgument] IFormatProvider? provider, InterpolatedStringBuilder builder);

and have the provider passed to the GetInterpolatedStringBuilder method.

I would like us to be able to add:

Utf8Formatter.TryWrite([InterpolatedStringArgument] Span<byte> destination, Utf8InterpolatedStringBuilder);

and have the span passed to the builder as the destination.

This also provides a solution for using stack scratch space. We can add an overload:

InterpolatedStringBuilder.GetInterpolatedStringBuilder(int baseLength, int formatHoleCount, Span<char> scratchSpace, out InterpolatedStringBuilder);

Then either we could expose a public API, or even if not internally use (and tell devs that want the interim solution to copy/paste it):

public static string Format([InterpolatedStringArgument] Span<char> scratchSpace, InterpolatedStringBuilder builder);

which we can then use like:

string result = Format(stackalloc char[256], $"{a} = {b}");

`await`s in the interpolation holes.

Alternatively, we could simply make all builder types non-ref structs, including the framework builder for interpolated strings. This would, however, preclude us from someday recognizing a `Span`
version that does not need to allocate any scratch space at all.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should say:

  • InterpolatedStringBuilder is a ref struct.
  • If you're targeting strings, the compiler choose between its various ways of implementing string interpolation: InterpolatedStringBuilder, string.Concat, and string.Format. If there's something that prevents it from using InterpolatedStringBuilder, well then it uses one of its other mechanisms.
  • If you're targeting a custom builder, then you're subject to the constraints of that builder. If the builder is a struct or class, great, you can use await in holes; have at it. If the builder is a ref struct, then you can't use await in holes, just as you can't use anything the builder doesn't have a TryFormat overload for.

Co-authored-by: Yaakov <[email protected]>
@FiniteReality
Copy link

Would it be possible to also pass the expression used to generate the value for the "holes"? The proposal currently uses logging as an example, but as far as I'm aware, most logging frameworks recommend you use structured logging, which means log messages look like LogTrace("User {id} did a thing", user.Id) as we have no way of associating a name with the format parameters. Exposing the original expression to the builder would mean that it would be possible to use these new interpolated strings with structured logging.

For example:

var user = await GetCurrentUserAsync();
_logger.LogTrace($"User {user.Id} logged in successfully");

// Compiles to:
var receiverTemp = logger;
TraceLoggerParamsBuilder.GetInterpolatedStringBuilder(baseLength: 47, formatHoleCount: 1, receiverTemp, out var builder);
_ = builder.TryFormat("User ") && builder.TryFormat(user.Id, "user.Id") && builder.TryFormat(" logged in successfully");
receiverTemp.LogTrace(builder);

The implementation of TryFormat could look then potentially something like this (with appropriate optimisations, of course!):

bool TryFormat<T>(T value, string expression)
{
    _structuredData[GetStructuredKey(expression)] = value;
    // store and format value
}


public ref struct SpanInterpolatedStringBuilder
{
public static void GetInterpolatedStringBuilder(int baseLength, int formatHoleCount, Span<char> span, out SpanInterpolatedStringBuilder builder)
Copy link
Member

@stephentoub stephentoub Mar 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether GetInterpolatedStringBuilder should return a bool.

The TryFormat methods can all assume that the compiler generated code won't call them if a previous TryFormat returned false. However, in a case like this builder, all of the TryFormats need to be prepared for the possibility that the initial configuration was flawed, e.g. that the baseLength > span.Length, and so they all need to have upfront if (!_success) return false; checks, because one of them might be called. If instead GetInterpolatedStringBuilder returned a bool, and the generated code could then be more like:

_ = GetInterpolatedStringBuilder(..., out var builder) &&
    builder.TryFormat(...) &&
    builder.TryFormat(...);

then the individual TryFormat methods would not need to check _success.

@333fred
Copy link
Member Author

333fred commented Mar 8, 2021

Would it be possible to also pass the expression used to generate the value for the "holes"?

@FiniteReality I would expect that that would simply fall out of CallerArgumentExpression work. The spec intentionally just says that overload resolution on the TryFormat method has to succeed. This allows all of the standard compiler features to just work ™️, without any modification. So if there are optional parameters filled in by one of the various types of Caller info attributes, I'd expect that they will naturally work.

@stephentoub
Copy link
Member

I would expect that that would simply fall out of CallerArgumentExpression work.

What would that look like in this case? Where would you put the [CallerArgumentExpression], and what would the compiler pass to it? Are you saying you'd put that on the GetInterpolatedStringBuilder method? Or are you saying you'd put it on whatever method accepts the builder as an argument, in which case in order to get that into the builder it would also need something like #4486 (comment), yes?

@333fred
Copy link
Member Author

333fred commented Mar 8, 2021

Where would you put the [CallerArgumentExpression], and what would the compiler pass to it?

public bool TryFormat<T>(T arg, [CallerArgumentExpression("arg")] expr = "") { ... }

It would just work :)

@stephentoub
Copy link
Member

stephentoub commented Mar 8, 2021

It would just work :)

I misunderstood the suggestion. I thought it was asking for the whole string to be passed in (e.g. "{a} = {b}", not just the individual pieces (e.g. "a", " = ", "b"). I see reading the original sample now it was asking for the individual pieces.

@333fred
Copy link
Member Author

333fred commented Mar 8, 2021

That's not how I read @FiniteReality's question.

@FiniteReality
Copy link

It would just work :)

I misunderstood the suggestion. I thought it was asking for the whole string to be passed in (e.g. "{a} = {b}", not just the individual pieces (e.g. "a", " = ", "b").

I was asking for the expressions for each interpolated segment. E.g. in $"The value is {x + y}" I would expect "x + y" as the expression.

Comment on lines +37 to +38
When an _interpolated\_string\_expression_ is passed as an argument to a method, we look at the type of the parameter. If the parameter type has a static method
`GetInterpolatedStringBuilder` that can invoked with 2 int parameters, `baseLength` and `formatHoleCount`, optionally takes a parameter the receiver is convertible to,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where will we handle all of the annoying questions here like:

  • What accessibilities on the static method do we allow?
  • Do the parameter names have to match?
  • What about static interface members ...

Do we want to do it here or later in a more detailed spec?

Comment on lines +37 to +38
When an _interpolated\_string\_expression_ is passed as an argument to a method, we look at the type of the parameter. If the parameter type has a static method
`GetInterpolatedStringBuilder` that can invoked with 2 int parameters, `baseLength` and `formatHoleCount`, optionally takes a parameter the receiver is convertible to,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May want to clarify a bit that receiver is the receiver to the original method call. Took me a bit to think about if it was to one of the other items.

var receiverTemp = logger;
TraceLoggerParamsBuilder.GetInterpolatedStringBuilder(baseLength: 47, formatHoleCount: 1, receiverTemp, out var builder);
_ = builder.TryFormat("this") && builder.TryFormat(" will never be printed because info is < trace!");
receiverTemp.LogTrace(builder);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider that in many cases here the out parameter is going to be a ref struct. Further, in several known cases, the receiverTemp will also refer to a ref struct. That means this really needs to think about the implications to the "lifetimes must match" rule in span saftey.

The recent proposal for span improvements goes into this rule in detail and that should shed a bit of ligh to on the consequences here.

https://github.com/dotnet/csharplang/blob/master/proposals/low-level-struct-improvements.md#provide-parameter-does-not-escape-annotations

This is quite esoteric but has real implications on the usability of APIs that have this pattern. Happy to spend 15 min to chat about the problems here.

@jaredpar
Copy link
Member

jaredpar commented Mar 9, 2021

@333fred

It would just work :)

I agree it should work for the TryFormat case. That is very analogous to say LINQ methods and I verified that we do support caller info attributes there.

Do you believe it should also work for GetInterpolatedStringBuilder?

Base automatically changed from master to main March 12, 2021 19:15
For simplicity, this spec currently just proposes recognizing a `TryFormat` method, and things that always succeed (like `InterpolatedStringBuilder`) would always return true from the method.
This was done to support partial formatting scenarios where the user wants to stop formatting if an error occurs or if it's unnecessary, such as the logging case, but could potentially
introduce a bunch of unnecessary branches in standard interpolated string usage. We could consider an addendum where we use just `Format` methods if no `TryFormat` method is present, but
it does present questions about what we do if there's a mix of both TryFormat and Format calls.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about it, the more I think we need to allow this. I don't have a strong preference for whether it's two different TryFormat/Format names, or whether we just allow TryFormat to be void-returning in addition to bool-returning, but I suspect just saying TryFormat can return void will be a little simpler as it avoids the ambiguity of what to do if there's both Format and TryFormat methods with the same arguments (since in C# you can't overload on return type alone).

The most common use of the builder will be InterpolatedStringBuilder, and if we don't do this, we'll be building an unnecessary inefficiency into the pattern from the get-go, with every call site being larger and requiring an unnecessary (though easily predictable) jump instruction.

```cs
public ref struct InterpolatedStringBuilder
{
public void GetInterpolatedStringBuilder(int baseLength, int formatHoleCount, out InterpolatedStringBuilder builder)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public void GetInterpolatedStringBuilder(int baseLength, int formatHoleCount, out InterpolatedStringBuilder builder)
public static void GetInterpolatedStringBuilder(int baseLength, int formatHoleCount, out InterpolatedStringBuilder builder)

public bool TryFormat(ReadOnlySpan<char> s)
{
if (s.Length >= _array.Length - _count) Grow();
s.AsSpan().CopyTo(_array);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
s.AsSpan().CopyTo(_array);
s.CopyTo(_array);

```cs
public class String
{
public static string Format(InterpolatedStringBuilder builder) => builder.ToString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll want to bury this instead as a static on InterpolatedStringBuilder.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also consider allowing such builder arguments to be ref, so that the implementation of the method can clear out the call site's reference, if it, for example, contains a pooled array or some expensive resource that should be cleared to avoid lifetime extension.

@333fred 333fred requested a review from a team as a code owner March 25, 2021 00:18
@333fred
Copy link
Member Author

333fred commented Mar 25, 2021

@jaredpar @stephentoub do you have any further feedback? I'd like to get this checked in so future comments can be done as PRs to the checked-in spec.

@stephentoub
Copy link
Member

stephentoub commented Mar 25, 2021

Thanks, Fred. Read through it again... some more feedback:

  • I think we should change the GetInterpolatedStringBuilder method to simply be named Create. It's now living on the builder itself, as a static factory, so we don't need a verbose name for it, and we use "Create" for other builder factories used by the language (e.g. async method builders). It could also be a ctor (though I don't feel strongly about it): the spec states that the factory is there to enable pooling, but a) the majority of builders are going to be structs (likely ref structs) rather than classes, and b) even if pooled class instances are desirable, the actual pooling could be done by a struct builder that just wraps the instances.
  • I think we should change that Create/GetInterpolatedStringBuilder method to return the builder rather than out'ing it. We should then allow for an optional out bool at the end of the signature, rather than always have an out builder and allowing for the return value to optionally be bool or void. In addition to being cleaner from a factory perspective, there's also small perf benefits to it. And I believe there are benefits related to current span safety rules, but Jared can comment on that further.
  • "We also provide a new string.Format overload, as follows:"... It makes sense for there to be a method the compiler calls for normal interpolation into a string, e.g. string s = $"{a} = {b}";, but I think we should just bury that on InterpolatedStringBuilder... so, move this method to be a static on InterpolatedStringBuilder rather than on string. There's no benefit to the user calling this method directly.
  • "Do we want to have builders for System.IFormattable and System.FormattableString as well?" I don't know what it would mean to have a builder for "IFormattable", and having a builder for a FormattableString doesn't add any meaningful value.
  • "Non-try version of the API"... I think the proposal should take a hard stance: TryFormat should be able to return void or bool. Let's just make that the proposal and change it upon LDM feedback if there's strong sentiment not to. There's no reason to further bloat the majority use case for building strings with unnecessary IL and branching.
  • "Incorporating spans for heap-less strings"... with support for passing arguments into the builder, a developer can achieve this manually, and I expect we'll expose a method that lets such a caller stackalloc'd span be passed into the builder. If we can do better in the future with built-in compiler support, great.
  • "Passing previous arguments to the builder"... the proposal should also take a hard stance here: being able to pass arguments from the method into the builder is important and should be enabled via attributes.
  • "And if so, what will a ref builder look like?"... we would say it doesn't affect the call site... you still pass $"..." and the builder that's generated is passed by ref (you're not actually passing the string "..." in to this position, anyway, so arguments about needing to specify ref for some kind of consistency seem moot). If someone is calling the method by manually passing in their own builder, then they'd of course need to use ref at the call site.

@333fred
Copy link
Member Author

333fred commented Mar 25, 2021

the majority of builders are going to be structs (likely ref structs) rather than classes

I'm really not certain that's true, personally. The async issues with such builders is going to be real. I don't foresee any use of ref struct builders for logging frameworks, for example. I'm not opposed to a rename to Create, but that feels like something that can be looked at after merge.

In addition to being cleaner from a factory perspective, there's also small perf benefits to it.

I would say that it feels like that's specifically not cleaner. It goes against the TryGet pattern that's well-established in C#. I'm also interested by your perf-benefits point: shouldn't it be better to return something that will always fit in a single register, than returning a struct that is potentially bigger than one register?

but I think we should just bury that on InterpolatedStringBuilder

There is an open question that addresses this.

being able to pass arguments from the method into the builder is important and should be enabled via attributes.

This was the question we didn't get to yesterday. There are still important things to debate around out-of-order execution.

Overall, I don't see anything at this point that prevents me from going ahead and merging this, with more followup to come later, so I'm going to do that.

@333fred 333fred merged commit 56cbb1e into dotnet:main Mar 25, 2021
@333fred 333fred deleted the improved-interpolated-strings branch March 25, 2021 16:22
@stephentoub
Copy link
Member

I don't foresee any use of ref struct builders for logging frameworks

Really? I'd expect, for example, some logging builders to use InterpolatedStringBuilder as an implementation detail, to reuse all of its support for pooled arrays and building up strings and the like. And that would lead to it also being a ref struct.

I would say that it feels like that's specifically not cleaner. It goes against the TryGet pattern that's well-established in C#.

I disagree. The builder is the primary thing being created, and the bool is optional... if there's no bool, it's odd for the builder to be out and for a factory method to hand back the builder as an out with a void return. Also, if this is really supposed to be about the Try pattern, then the method should be prefixed with Try. And this method is also meant to be called by the compiler, not by users, so basing it off of patterns focused on usability is misaligned.

There is an open question that addresses this.

In multiple cases, the proposal basically says "we should do X", and then has an open questions section that says "but maybe we should also consider Y". Some of my points are saying to swap that, just say we're going to do Y, and if you feel you must, also offer as an alternative not doing so.

Overall, I don't see anything at this point that prevents me from going ahead and merging this, with more followup to come later, so I'm going to do that.

Some of the things were things I commented on two weeks ago and weren't addressed. But, fine. I'll look forward to your follow-up PR.

@333fred
Copy link
Member Author

333fred commented Mar 25, 2021

Really?

Really. I strongly believe that logging frameworks are going to look at this can go "Yeah, but my users can't put awaits in their logged messages, so the built-in version is useless to me."

In multiple cases, the proposal basically says "we should do X", and then has an open questions section that says "but maybe we should also consider Y". Some of my points are saying to swap that, just say we're going to do Y, and if you feel you must, also offer as an alternative not doing so.

It's a proposal. We'll need to go through all of these questions. I have no problem as presenting it as "the proposal currently says we should do X, but I think we should do Y."

@stephentoub
Copy link
Member

stephentoub commented Mar 25, 2021

Really. I strongly believe that logging frameworks are going to look at this can go "Yeah, but my users can't put awaits in their logged messages, so the built-in version is useless to me."

Can you point to some real-world examples of logging code that has tons of awaits in holes in interpolated strings?

@333fred
Copy link
Member Author

333fred commented Mar 25, 2021

Can you point to some real-world examples of logging code that has tons of awaits in holes in interpolated strings?

Talking with @maryamariyan, it seemed very likely that they would not be able to use a ref struct for these reasons. Remember that it's not just in the holes for non-string types: you can't have an await in the entire containing statement, because the builder will be around for that entire statement, as opposed to the string case where it will only be around for as long as is necessary to build a string.

@333fred
Copy link
Member Author

333fred commented Mar 25, 2021

you can't have an await in the entire containing statement, because the builder will be around for that entire statement, as opposed to the string case where it will only be around for as long as is necessary to build a string.

I'm also not particularly convinced we should allow these things in async method bodies at all, because of the potential for user confusion. For generating the code that turns an interpolated string into a regular string, I can see us optimizing that when there are no awaits in any of the holes, but I'm concerned about the inconsistency and usability of interpolated strings converted to ref structs in async methods if we allowed piecemeal usage in constrained scenarios. I'd want a more general "you can use a ref struct in an async method as long as you're not awaiting when you use it" type feature for that.

@stephentoub
Copy link
Member

stephentoub commented Mar 26, 2021

Talking with @maryamariyan, it seemed very likely that they would not be able to use a ref struct for these reasons.

I'm surprised by that, since the strongly-typed API / source generator being discussed in dotnet/runtime#36022 (and for which I suggested she speak with you about using the new interpolated string support instead) had everything guarded behind method calls, with all of the arguments eagerly computed, e.g. a method signature exposed like:

[LoggerMessage(0, LogLevel.Information, "Hello '{name}'")]
public partial void LogName(string name);

and if that's the exposed signature, there's zero problem with awaits being used in that call, since if it were to be implemented with interpolated string builders, that would be in the implementation of this method, e.g. instead writing:

public void LogName(string name) => _logger.Log(0, LogLevel.Information, $"Hello '{name}'");

as I sketched here:
dotnet/runtime#36022 (comment)

you can't have an await in the entire containing statement, because the builder will be around for that entire statement

Can you elaborate with an example?

I'm also not particularly convinced we should allow these things in async method bodies at all

I don't understand. What "things"?

I'm also still waiting for real examples where awaits are used in holes inside of interpolated strings. I've literally never seen one. I'm sure someone somewhere has done it, but it's in no way common in my experience.

@333fred
Copy link
Member Author

333fred commented Mar 26, 2021

Can you elaborate with an example?
I don't understand. What "things"?

Ref struct builders at all. For example, take await a.B($"{c}"); The call to B is awaited, and we won't be able to safely use a ref struct builder for that interpolated string because of it. Or a.B($"{c}", await D());. Similarly here, we can't spill that ref struct builder and await the call to D(). And neither of these cases consider that, as you've mentioned previously, we may want to pass such builders by ref, rather than by value, and we may to ensure that such builders are disposed after usage. I'd rather look at a feature that enables general ref struct usage in async methods, when safe to do so, rather than try and work out lifetimes for just this special context.

@stephentoub
Copy link
Member

stephentoub commented Mar 26, 2021

Ref struct builders at all. For example, take await a.B($"{c}"); The call to B is awaited, and we won't be able to safely use a ref struct builder for that interpolated string because of it. Or a.B($"{c}", await D());

Why? Nothing from those interpolations would span an await. What am I missing?

@333fred
Copy link
Member Author

333fred commented Mar 26, 2021

I'm surprised by that, since the strongly-typed API / source generator being discussed in dotnet/runtime#36022 (and for which I suggested she speak with you about using the new interpolated string support instead) had everything guarded behind method calls, with all of the arguments eagerly computed

From talking with her offline, there's 2 scenarios they're concerned with:

  1. Reusable, strongly-typed templates. This is not something that this proposal is particularly well-equipped to deal with. Yes, we might be able to help simplify the LogName method generation, but it's overall not really saving much, as this proposal is mainly targeted at the end user writing code, not a generator that can easily generate one set of boilerplate instead of another.
  2. One-off templates. These wouldn't have any separate generation or methods behind them, but would instead be directly where the user wants to log. This means LogInfo($"My {name}"); directly in the method itself, not in some other method.

@333fred
Copy link
Member Author

333fred commented Mar 26, 2021

Why? Nothing from those interpolations would span an await. What am I missing?

For 1, the ability to dispose or otherwise ensure that any pooling and cleanup is taken care of. For 2, order of evaluation :).

@stephentoub
Copy link
Member

stephentoub commented Mar 26, 2021

For 1, the ability to dispose or otherwise ensure that any pooling and cleanup is taken care of. For 2, order of evaluation :).

Please write out the code. I do not see the problem.

Your example was:

await a.B($"{c}")

That becomes:

A a = ...;
SomeBuilder b = SomeBuilder.Create();
b.TryFormat(c);
await a.B(b);

or if you want to factor in the idea of making SomeBuilder disposable:

A a = ...;
SomeBuilder b = SomeBuilder.Create();
b.TryFormat(c);
Task t = a.B(b);
b.Dispose();
await t;

What is the problem there?

@stephentoub
Copy link
Member

stephentoub commented Mar 26, 2021

Reusable, strongly-typed templates. This is not something that this proposal is particularly well-equipped to deal with.

I don't see why not. The proposal was to write:

[LoggerMessage(0, LogLevel.Information, "Hello '{name}'")]
public partial void LogName(string name);

and have a source generator that would fill in details of LogName. With a builder instead, the developer still writes all the same information, just without the need for the source generator, with the compiler handling the parsing, etc.:

public void LogName(string name) => _logger.Log(0, LogLevel.Information, $"Hello '{name}'");

And await doesn't impact that at all, because any awaiting would be at the call site.

One-off templates. These wouldn't have any separate generation or methods behind them, but would instead be directly where the user wants to log. This means LogInfo($"My {name}"); directly in the method itself, not in some other method.

Great. Where are the real-world examples where await is used in the holes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants