proposal: auto closure

[darrenoakey]

I'm doing a lot of swift at the moment - and one thing that I really like is @autoclosure - would love something similar in C#.
It's the sort of thing you'd only use in a few cases, but those would have wide ranging affects.. Particularly of course - log.info/debug/whatever. everyone knows that you are meant do do the whole "if log.debug... " or pass in a delegate, so if logging is off, you don't do any work... but of course no one ever does that.... being able to declare autoclosure on the log parameters would mean the library can make that decision.... AND you can guarantee for instance that a logging line can never throw an exception.

[canton7]

Perhaps describe what you want in C# terms, rather than expecting people to go and read the Swift docs?

[darrenoakey]

sorry - the idea is - there are times, especially log statements, debug statements, assertions etc - which you use a lot, so you want the most convenient possible syntax - but ideally you'd pass a closure to - so you can do things like defer work or trap errors.

so the idea would be that I can write a method like

   class Log 
   {
          public void Info( [AutoClosure] Func<String> message )
          {
                if (LogInfo.IsEnabled)
                {
                       try { 
                               _underlying.log( Level.info, message() );
                       } catch (Exception theException)
                      {
                           // log methods should never throw
                      }
            }

but I can just call it like this:

        Log.info( "hello " + " world");
        Log.info( $"the object= {this}");
        Log.info( () => "hi");

and in each case the compiler, when it gets to the parameter marked with [AutoClosure] - effectively just wraps whatever's there in a closure - so for instance if info is not enabled, no strings get added, no testing gets called etc

[svick]

So it's just about omitting () => from the code? I'm not sure that's worth it, especially since it makes understanding the code harder.

[darrenoakey]

it sort of is about getting rid of () =>.... but first, if you are talking something like log.info, or every assert - it's about getting rid of it in thousands of places... but really it's more about finding a way of getting both convenience and good behaviour without having to choose.

we've all written log libraries - and stuff that uses log libraries. you know that you aren't meant to do work in a log line, you know ideally that every value should be either a constant string or a closure, and in many cases you want to do all the work on another thread so you don't affect the critical path....

but no-one actually does that. People do Log.info( $"the object = {this}" ). And it ends up doing a lot of work. On the critical path, and it's not completely obvious. for example, a year ago one of the algos we were involved with was having some perf problems, and they found that like 50% of the entire time spent in the algo was in double.toString() because of logging. (This was in Scala)

Also - if you are talking about understanding the code, I'd argue

    Log.info( () => ("hello" + " world") )

has a much greater cognitive load than

    Log.info( "hello" + " world")

and it occurs in thousands of places in your code - whereas the autoclosure directive appears like once... so the idea is to make the code cleaner and easier to understand.

a feature like the above seems minor - but it allows you to make a library function that forces the user to be well behaved, without forcing them to do extra work. But more than that - while it IS a minor feature.... it's also a minor change - its like the [callermethodname] and stuff.... it doesn't change the shape of the language at all - it's not introducing any new ideas - all it means is that when the compiler sees an attribute at a call point it emits a few extra statements - there's no "wider" impact or implications to worry about - so while it may be a minor win, it's also a minor cost - and I'd say the benefit way outweighs the cost?

[333fred]

Also - if you are talking about understanding the code, I'd argue

    Log.info( () => ("hello" + " world") )

has a much greater cognitive load than

    Log.info( "hello" + " world")

and it occurs in thousands of places in your code - whereas the autoclosure directive appears like once... so the idea is to make the code cleaner and easier to understand.

I wouldn't agree with that assessment. Now, everywhere I see a method call, I have to wonder whether it's getting implicitly converted to a closure. This can have significant effect on the generated code: locals get hosted to display classes, allocations are created, and lifetimes change. I believe it would be very easy to create bugs in this fashion by accidentally missing that a method parameter is an autoclosure.

[TheUnlocked]

My concern is that it's not that uncommon to want to log the return value of a function with side effects. Say there's some query language whose implementation has a function ActionQuery() that returns a string describing the status of the input query:

Log.info(db.ActionQuery("create table foo"));
Log.info(db.ActionQuery("add thing to foo"));
var thing = db.GetQuery("* from foo");

In this example, it looks like the third query should be safe, but if evaluation of the first two is deferred (or just doesn't occur at all), the table it's trying to access might not even exist yet! This is an issue which can't be caught at compile time and which could lead to hard-to-detect errors.

[theunrepentantgeek]

People do Log.info( $"the object = {this}" ). And it ends up doing a lot of work. On the critical path, and it's not completely obvious.

It's orthogonal to the feature you're proposing ... but any worthwhile Log.Info() method would likely take a FormattableString and defer the cost of building the string until it was actually needed, minimizing the cost of logging that's not used.

When you use $"interpolated string {syntax}" it renders to a FormattableString and then ToString() is called only if needed.

[darrenoakey]

It's orthogonal to the feature you're proposing ... but any worthwhile Log.Info() method would likely take a FormattableString and defer the cost of building the string until it was actually needed, minimizing the cost of logging that's not used.

it's not orthogonal - that is exactly the point - if we get this feature, then the "work" involved in the interpolated string won't happen unless it's needed (and you would hope that string interpolation will always be better than or as good as a Formattablestring, because it can do some bits at compile time?)

[darrenoakey]

My concern is that it's not that uncommon to want to log the return value of a function with side effects.

is it really that common? for us it's very strictly against the rules. In fact I've toyed with the idea of making something that deletes all the log lines - and runs the entire test suite with and without logging.

but basically - if you are in a world where that's ok... then don't use AutoCloseable when you are writing your log function :)

[canton7]

It's orthogonal to the feature you're proposing ... but any worthwhile Log.Info() method would likely take a FormattableString and defer the cost of building the string until it was actually needed, minimizing the cost of logging that's not used.

Note that this still needs to allocate the FormattableString object, and box any value-type parameters. It's likely that this won't be must of a cost-saving, and indeed it was never intended as such.

[theunrepentantgeek]

and you would hope that string interpolation will always be better than or as good as a Formattablestring

Interpolated sting's compile into use of FormattableString, except (I think) when the compiler can compile it into a totally constant string.

[canton7]

Interpolated strings normally compile into string.Format, unless the compiler can optimize it to string.Concat. They only compile to FormattableString if you explicitly tell the compiler to.

[canton7]

For fun, I ran some benchmarks.

Method and detailed results

[MemoryDiagnoser]
[RyuJitX64Job]
public class MyBenchmark
{
    private readonly double d = 1.2;
    private readonly int i = 4;
    private readonly string s = "hello";

    [Benchmark]
    public string Interpolated()
    {
        int local = 3;
        return LogPlain($"{d}{i}{s}{local}");
    }

    [Benchmark]
    public string InterpolatedStringOnly()
    {
        string local = "3";
        return LogPlain($"{s}{s}{s}{local}");
    }

    [Benchmark]
    public string FormattableNotUsed()
    {
        int local = 3;
        return LogFormattableNotUsed($"{d}{i}{s}{local}");
    }

    [Benchmark]
    public string FormattableUsed()
    {
        int local = 3;
        return LogFormattableUsed($"{d}{i}{s}{local}");
    }

    [Benchmark]
    public string ClosureNotUsed()
    {
        int local = 3;
        return LogClosureNotUsed(() => $"{d}{i}{s}{local}");
    }

    [Benchmark]
    public string ClosureUsed()
    {
        int local = 3;
        return LogClosureUsed(() => $"{d}{i}{s}{local}");
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static string LogPlain(string message) => message;

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static string LogFormattableNotUsed(FormattableString message) => null;

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static string LogFormattableUsed(FormattableString message)
    {
        return message.ToString();
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static string LogClosureNotUsed(Func<string> closure) => null;

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static string LogClosureUsed(Func<string> closure) => closure?.Invoke();
}

class Program
{
    static void Main(string[] args)
    {
        var summary = BenchmarkRunner.Run<MyBenchmark>();
    }
}

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.18363.959 (1909/November2018Update/19H2)
Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=5.0.100-preview.7.20366.6
  [Host]    : .NET Core 3.1.4 (CoreCLR 4.700.20.20201, CoreFX 4.700.20.22101), X64 RyuJIT
  RyuJitX64 : .NET Core 3.1.4 (CoreCLR 4.700.20.20201, CoreFX 4.700.20.22101), X64 RyuJIT

Job=RyuJitX64  Jit=RyuJit  Platform=X64  

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Interpolated	342.89 ns	6.332 ns	5.923 ns	0.0420	-	-	176 B
InterpolatedStringOnly	27.04 ns	0.606 ns	0.809 ns	0.0134	-	-	56 B
FormattableNotUsed	33.91 ns	0.716 ns	0.931 ns	0.0382	-	-	160 B
FormattableUsed	376.36 ns	7.256 ns	8.065 ns	0.0496	-	-	208 B
ClosureNotUsed	12.42 ns	0.260 ns	0.243 ns	0.0229	-	-	96 B
ClosureUsed	369.21 ns	6.330 ns	5.921 ns	0.0648	-	-	272 B

The cheapest options if you want to disable logging (in order from cheapest):

1. A closure (12ns)
2. Interpolated strings which can be lowered to string.Concat (or just using string concatenation directly) (27ns)
3. A FormattableString (33ns)
4. Interpolated strings which have to be lowered to string.Format (349ns)

If you actually want to turn your log message into a string, the cheapest options are:

1. Interpolated strings which can be lowered to string.Concat (or just using string concatenation directly) (27ns)
2. Interpolated strings which have to be lowered to string.Format (349ns)
3. A closure (369ns)
4. A FormattableString (376ns)

Basically, if you're only concerned with how fast your logging calls are when you disable logging, closures are an OK approach (although not that much cheaper than just using a string.Concat / FormattableString). If you want to consider how long it takes to generate your log message with logging turned on, closures aren't quite so tempting. FormattableString beats an interpolated string which can't be lowered to string.Concat if a log message isn't needed, but isn't great otherwise.

I think effort would be better put into speeding up interpolated strings (there are constant ongoing discussions here).

[darrenoakey]

although it worries me that we've gone off on a logging tangent - this isn't just about logging.

and it's also not just about performance - it's also about safety ... one of the reasons you might want to put something in a closure is to defer it - but there are many other reasons.... for instance - to prevent it throwing (you never want the program being brought down by a log line) - or to add synchronisation around it. Hey - you could even use this mechanism to implement an Erlang actor pattern - and basically punt the closure to a queue in a threaded work pool.

In swift, for instance - where I encountered this attribute - one of the reasons they use it is in Asserts. Every expression passed to the assert turns into a closure so they can capture the exception - so even if you say something like AssertEquals( "green", bob.ToString()) - and bob doesn't toString well - you don't get an exception thrown from the test- you get an interesting assertion failure error saying that the exception string doesn't match "green"

[darrenoakey]

For fun, I ran some benchmarks.

love it

one thing though - not to take away at all for testing that - it's very useful to know - is the reason I put in "this" - is I was just trying to give an example of "work". this.toString() could take an hour for all we know. I'm not necessarily assuming the formatting itself is all that heavy - as you've demonstrated.

[canton7]

I don't think [AutoClosure] is about safety. The language already has a perfectly safe way to declare that a delegate should be passed to a method, and that's for the method's signature to declare that it accepts a delegate.

If you want your logging methods to only accept delegates, then make sure that their methods only accept delegates.

This is purely about convenience as far as I can see: saving a couple of extra characters when passing a delegate to a method, at the expense of clarity.

[Flutterish]

public void Info( Func<string> message ) { ... }
public void Info( string message ) => Info( () => message );

Use the delegae overload when you know you need to do work.

[darrenoakey]

public void Info( Func<string> message ) { ... }
public void Info( string message ) => Info( () => message );

Use the delegae overload when you know you need to do work.

that's the entire point - the reason for this request - is because that's the code you've written is what everyone does - they create the above overload. And that's the problem - because 99.9999% of the time, the user just uses the string overload without thinking about the implications, because it's easier, and they are just adding a log statement quickly because they are troubleshooting something, and particularly all sort of work can be hidden behind the innocent seeming implicit "ToString()"

The entire point of this request is to take the burden of choice away from the user.
Mind you - this IS something we currently can (and do) with fody or postsharp.

[canton7]

Why not just remove the string message overload then? That way the user can't call it by accident.

[darrenoakey]

religion I guess - I have two beliefs that come into play here - I believe that an api should be written using the "principle of least surprise". People expect to be able to say Log.Info("hello world") - and will get confused and annoyed if that overload doesnt exist.
The second belief is that there is literally no limit to the amount of nastiness it's appropriate to sweep under the carpet to make an external api even slightly cleaner or easier to use. Log.info gets written once, but gets used thousands of times, and gets read many times more than that. Log.info($"trade={trade}") is easier to write than Log.info( () => $"trade={trade}" ) and far easier to read. If you can save each person a few moments every time they look at a piece of code, like that classic xkcd comic - it adds up quickly.

[canton7]

I believe that an api should be written using the "principle of least surprise"

I think people would be surprised if they wrote Log.Info("hello world"), and the compiler turned that into Log.Info(() => "hello world");. I think they'd be surprised if they used an overload which explicitly takes a string, and the compiler silently rewrote that to turn it into a delegate.

People expect to be able to say Log.Info("hello world") - and will get confused and annoyed if that overload doesnt exist.

Note that it still won't exist -- intellisense will continue to only show the Func<string> overload, because that's the overload which exists (and attributes aren't shown in intellisense).

Log.info($"trade={trade}") is easier to write than Log.info(() => $"trade={trade}") and far easier to read.

So this is a discussion about removing 6 characters. The language has never had terseness as a goal: expliciteness and clarity have always been more important.

@333fred has given their opinion. In the absence of anyone else from the LDM chipping in, I think their concerns are probably the ones you need to address.

---

We touched on FormattableString, but it's worth repeating. This code:

void Info(FormattableString fs) { }

Info($"trade={trade}");

will not call trade.ToString(). That only happens if and when fs.ToString() is called. Does that not solve your issue?

[darrenoakey]

i would want intellisense to be aware of this - and show the function as a function that takes a string - just like the existing one that already exists in swift works:

https://www.swiftbysundell.com/articles/using-autoclosure-when-designing-swift-apis/

and it's not about removing 6 characters - it's about "cognitive load" - I reckon however familiar you are with a language, it just takes more mental effort to parse what's going on with blah( () => "green" ) than it does to parse blah( "green" ) - and in any program, for me the priorities always are a) error handling and then b) ease of readability

as for surprise - the user doesn't need to know what happens inside log.info, as long as it works. Modern compilers and optimizers do all sorts of crazy stuff with code - look at llvm - which would surprise most people looking at the generated result - I only care about how easy and obvious my api is to use - nobody needs to care what bizarre things it has to do to get that way, once the final result is generated. We use weavers (fody/postsharp/manifold) heavily, so often just completely rewrite code - it's all about being able to express intent in the most obvious way possible.

[jmarolf]

This is something I see requested a lot. Normally in the context of linq. Basically the problem is people do not want to repeat () => or _ => all over the place in their code. While I am sympathetic I have not seen a proposal for this that I think checks all the boxes (for me at least).

@darrenoakey just so I make sure I understand you. Your use case is that you want create logging strings lazily so they do not allocate at all in the event of a release build of the product never realizing them. Currently passing in a delegate is the canonical way to do this in C# and as @333fred says I think it does tell you something important: mainly that a closure is going to capture all the current state and potentially run later. This would tell me not to do Log.Info(() => $"{veryLargeObject}"); as that object will be held in memory by the closure.

[darrenoakey]

actually - there are a number of reasons autoclosure, I just picked logging as a well understood example.

In my personal case, we always have the same level of logging in production as in test, so it's not about turning things off for the release build - for me with logging the "nice to have" is being able to do the work asynchronously - but the primary win is to be able to insert a "try catch" around the contents of the log call - and guarantee that the log line can never throw - so even if you did Log.info( myFunction.ThatAlwaysThrows() ) - it would just happily log the error, and not crash the program - as I sort of believe log statements are there for troubleshooting, they don't form part of the main codebase - so you should never be in the situation where core functionality fails because someone adds a log line.

And I get what you are saying - but I think there are two alternate ways of looking at programming languages -
a) a set of constructs that codify and simplify writing the production of byte code or
b) a way of expressing what you want the computer to do.

for many years we've been stuck in in the "how it does it" - but to me the ideal end goal is I can type "alexa - build me a program that's pretty much like photoshop but is more in the UI style you know I prefer and incorporates the super-resolution techniques from that article I read a few days ago"...

I think about everything in this forum is incremental steps towards that eventual end goal, and greater the difference between what we type and what gets produced, the more value the language brings to the table.

[theunrepentantgeek]

@darrenoakey wrote

... it's not about removing 6 characters - it's about "cognitive load" - I reckon however familiar you are with a language, it just takes more mental effort to parse what's going on with blah( () => "green" ) than it does to parse blah( "green" )

Cognitive load is exactly why I think any feature remotely close to this would be a real problem.

At the moment (as of C# 8) when I call blah("green") I know that it's passing a string to the method.

And when I see blah(()=> "green") I know that it's passing a delegate that might be cached and called later.

Those extra characters aren't a waste of space - they accurately flag that the parameter being passed is a very different kind of critter, and I need to be aware of what's going on (e.g. for object liveliness wrt garbage collection).

If the compiler were to silently convert from blah("green") to blah(()=> "green") then every other function call would be suspect; I'd have to evaluate the signature and documentation for every other method in case it used an autoclosure. I wouldn't be able to rely on my memory either - in case the method had been changed by another developer or a library update.

This would dramatically add to the cognitive load of reading code, and not just for code that uses autoclosures - it would impair the ability of any developer to reason about the code they're reading.

[darrenoakey]

I guess this shows a vastly different way we think about things - for me the main point of something like an autoclosure - is that I want the library writer to be able to take a decision away from the end user. Not only do I not want the user of my library to care that it's being turned into a delegate - ideally, I don't even want them to know. it's really none of their business.

I think of software like lego - you build opaque lego blocks that snap together simply, and if you can correctly snap them together, then they should work - I just want to give a library writer the ability to think about something once, so the library users don't have to every time they use it. If a writer uses autoclosure, then they have thought through why they are doing it - their consumer shouldn't care.

but anyway - I suspect that is probably just an opinion that many people would disagree with. I guess I mostly write libraries for myself, and I'm often both lazy and careless. to me, it's simple - on one hand - If I end up locking some big object in memory, then maybe one day it will increase the load on the gc, and that maybe in a speed sensitive area, and maybe it will slow down something enough to care about and maybe one day I'll need to do some profiling and fix it... on the other - maybe I add some log line that does a tostring on something that crashes, so submitting a trade fails and we lose $hundreds of thousands. The risk I'm introducing doesn't compare in any way to the risk I'm removing.

But either way - just because some magic rewrite functionality exists in code doesn't mean I'll ever be "watching out" for it - because I'll assume (most of the time correctly) that if someone IS using it, they know what they are doing, and it won't affect me.

as a postscript - you mention "any feature remotely close to this would be a real problem." - like it's a really dangerous road that no one would go down - remember, I'm just asking for a feature I can (and do) use today in Swift. ie - other languages designers have already both decided on and implemented it. Yes, languages have points of view, no we shouldn't do everything everyone else does - but I almost think we should consider everything anyone else puts in their language like - ie it should go like "language X just added feature Y - so it obviously fulfils a need - do we have a better way to achieve it? does it just not fit with our language paradigm? Does it conflict with anything we have done or are doing? if no, no and no, maybe we should add it?" the whole reason I use C# for fun over java which I have to use as a job is because C# is constantly growing.

[AartBluestoke]

@darrellbooker would autocloser mean that the users of your hypothetical library would not be able to reason about variable state of the functions?

{ myLocalVar = bar("thing");
foo(myLocalVar);
myLocalVar = bar("thing");
}

is there any chance that foo would operate on the thing2 version?

because

{ myLocalVar = bar("thing");
foo(()=>myLocalVar);
myLocalVar = bar("thing");
}

makes it clear that myLocalVar is captured, and foo could use either version.

[CyrusNajmabadi]

is that I want the library writer to be able to take a decision away from the end user

This would be an anti-goal for me.

but the primary win is to be able to insert a "try catch" around the contents of the log call

I would not want this. You are now changing the semantics of my code. If I write Foo(Bar()), and Bar throws, then I better get an exception.

I want the separation between caller and called to be explicit. If the callee wants to change things, that is done clearly by explicitly making a lambda and clearly then passing in the code block to the callee and not the value itself.