dotnet / csharplang

The official repo for the design of the C# programming language
11.49k stars 1.02k forks source link

Explicit Local Methods #1882

Closed SilentSin closed 6 years ago

SilentSin commented 6 years ago

We've got Explicit Interface Methods such as:

void IDisposable.Dispose()
{
}

Which can only be called directly on an IDisposable variable.

And we've got Local Methods such as:

private void MyMethod()
{
    MyLocalMethod();

    void MyLocalMethod()
    {
    }
}

Which can only be called from within their declaring method.

So what if we could combine the two concepts:

private void MyMethod()
{
    MyLocalMethod();
}

void MyMethod.MyLocalMethod()
{
}

This gives the same control over where MyLocalMethod can be called from, but avoids the need to cram it all into MyMethod.

You would of course lose the ability to create closures of such methods.

Motivation:

By restricting the exact caller of a method, that method can more safely make assumptions about the conditions in which it will be called and we can avoid polluting intellisense with methods that are only meant to be called in one place, all without making the main method massively large by filling it with locals.

Mechanics:

Functionality-wise, I'd expect this to be just syntactical sugar that compiles into the same thing as a local method.

Breaking Change?

Only if someone theoretically had something like this:

class MyClass : IDisposable
{
    public void IDisposable()// WTF kind of method name is that.
    {
        Dispose();
    }

    void IDisposable.Dispose()
    {
    }

    public void Dispose()
    {
    }
}

My recommendation would be an ambiguity warning which will otherwise still compile to use the public version that it would have before.

CyrusNajmabadi commented 6 years ago
  1. How would you create a local function for an explicitly implemented method?
  2. What happens if you have multiple overloads of a method? How do you express which overload the local function belongs to?
  3. It's not clear what problem this is solving. Your example, prior to the rewrite, seems utterly fine. It works and works well. It doesn't seem to have anything about it that actually gets better here.
SilentSin commented 6 years ago
  1. An explicitly implemented IDisposable.Dispose could have access to Dispose.Whatever just like any other function called Dispose and/or it could allow nesting of IDisposable.Dispose.Whatever. It comes down to whether you'd see the feature as a specific single method targeting system or just as a way to keep it out of the general intelisense list without being super restrictive.
  2. Similar to number 1. Either all overloads of Dispose can call methods prefixed with Dispose. or when you implement Dispose.Whatever you can specify the parameters of the overload it can be accessed from like Dispose().Whatever.
  3. That example seems fine because it's a single empty example. This feature would let you take any large method and split into multiple parts without making all those parts accessible to the rest of the class. For example:
    
    public void Serialize()
    {
    using (var file = new FileStream(...
    {
        WriteHeader(file);
        WritePartA(file);
        WritePartB(file);
        WritePartC(file);
        WriteFooter(file);
    }
    }

void Serialize.WriteHeader(FileStream file) { }

// Etc.


You don't want to call `WriteHeader` from anywhere else but you also don't want to turn `Serialize` into a massively long function. Maybe you want `WriteHeader` to even be in a different `#region` (or even a different file in a partial class) with the rest of the stuff it relates to, but while still clearly indicating where it is used.
DavidArno commented 6 years ago

Dup of #1329.

This proposal suffers from all of the problems in the original proposal too:

  1. Overloading issues,
  2. How to handle closures
  3. Such functions are no longer local functions; they are restricted scope private methods.

As @aluanhaddad says in that other proposal, "The whole point of a local function is that it's local". They are scoped to a method and live within that method, not somewhere else inside a class or struct.

SilentSin commented 6 years ago

You're right that it's a duplicate, but I don't particularly see any of those as problems.

  1. I suggested two ways overloads could be handled in point 2 in my previous comment. Allow the method to be called from any method with the correct name (void OuterMethod.InnerMethod()) because every OuterMethod likely has a very similar purpose and/or specify the parameter types of the exact overload you want (void OuterMethod(float).InnerMethod()).
  2. Just don't handle closures. Local methods have access to parameters depending on their location in the enclosing method. Methods with this proposal aren't actually in that method and don't even have to be near it in the file so they shouldn't have closures at all. Local methods allow for closures while this feature allows you to split large methods without making the parts accessible to the rest of the class. They each have different strengths.
  3. Yes they are, so ... it needs a different name? Restricted Private Methods sounds fine. Targeted Methods? Not Really Local Methods? Neighbour Methods? Friend Methods? No, wait, here's a good one: since they're like Explicit Interface Methods we should call them Explicit Method Methods. :)

I totally agree with your last comment in that other thread; if you need a #region inside a method, it's doing too much. But that method obviously has a purpose and that stuff all needs to get done somewhere, so it's just a matter of your available options for refactoring:

And as an added bonus you'd be able to hit F12 in VS on OuterMethod to go straight to it, knowing that's the only place InnerMethod is called instead of needing to find all references to InnerMethod and select the one you want. That's like, two whole seconds saved every time. Imagine all the spare time you'll have.

SilentSin commented 6 years ago

Another point in that last list:

SilentSin commented 6 years ago

@DavidArno

As @aluanhaddad says in that other proposal, "The whole point of a local function is that it's local". They are scoped to a method and live within that method, not somewhere else inside a class or struct.

That's not an argument against this feature, nor even an argument at all, it's just a description of the existing feature. The whole point of an Explicit Not Really Local Method is that it's linked to a method without being required to live within that method, allowing it to be located wherever it makes the most sense.

In the serialization example I gave earlier WriteHeader could be located near ReadHeader, DrawHeader, and whatever other related stuff the class has, even though each of those methods is only meant for one specific call site. Looking at and reading the Serialize method is just as easy as any method that calls a bunch of others (literally the same as if all those others were regular privates), but looking at those other methods makes it much clearer what they are used for. Like how a system diagram might want to show a 1 to 1 relationship instead of needing either 1 to many or literally putting one of the elements inside the other.

CyrusNajmabadi commented 6 years ago

That example seems fine because it's a single empty example. This feature would let you take any large method and split into multiple parts without making all those parts accessible to the rest of the class. For example:

This example doesn't seem interesting either. I don't see anything wrong with the local function version of it:

You don't want to call WriteHeader from anywhere else but you also don't want to turn Serialize into a massively long function

The function isn't massive long. It's effectively teh same length as before. It just the final curly is written on a different line. I don't see anything substantively different.

CyrusNajmabadi commented 6 years ago

Basically, as before, i don't see what actual problem this is solving. The strongest hting i've heard so far is that it allows the impl to just go anywhere (in the containing class), potentially even in a different 'partial' part. However, this doesn't actually sound like a good thing to me.

If that were allowed, i'd start wondreing, "why can't i declare a class method outside of a class? why can't i just put it in an arbitrary namespace-part somewhere in my project?" Basically, the idea of having something scoped narrowly be allowed to just float around anywhere doesn't actually seem like a good thing to me. It actively seems like something i would not want to be allowed.

So this feature actually sounds like a large negative to me. Not a positive. The very restriction it is circumventing is a restriction i think is super valuable.

CyrusNajmabadi commented 6 years ago

Other concerns:

I can place local functions in nested scopes. (i.e. instead an 'if' block, and whatnot). How would i do that here? Would this literally just be for non-capturing, top-level, local-functions for a method?

Also, how would you handle this for a constructor, destructor, property-getter/setter, or any other places where you could put a local function?

SilentSin commented 6 years ago

The function isn't massive long. It's effectively teh same length as before. It just the final curly is written on a different line. I don't see anything substantively different.

It seems to me that you're having trouble seeing the forest through all the trees. Rather than getting caught up in whether or not the intentionally brief and generic examples I've given fit your definition of "too long", consider any method you have encountered that you would personally put in that category, I'm sure you've seen some. Or not even that, consider any situation where you've decided - for whatever reason - to move part of a method out into another private method.

Why not go for a public method? Because privates offer better safety against mistakes by other users (including future you).

What does this feature offer? Even better safety than privates.

Basically, as before, i don't see what actual problem this is solving.

Rather than thinking of this feature as "local functions without closures that can be scattered all over the class to create a huge mess", instead think of it as "private methods with additional safety restrictions". Defining methods outside the class or out in a namespace probably has merit for some people, but that's the exact opposite of what I'm after.

The problem I see is that there is currently no middle ground between "this can be accessed anywhere in the class" and "this is actually just part of the outer function but I've rearranged it down the bottom because I can".

This is similar to the arguments I made in favour of persistent local variables (https://github.com/dotnet/csharplang/issues/832#issuecomment-416159872):

Nested classes achieve lazy initialisation, but they can't control access. Kind of like how local functions can't be called by outsiders and are good for closures, local member variables can't be touched by outsiders and are good for lazy initialisation. A simple if null then make new check is enough for most lazy initialisation, but it's the ability to ensure that nothing else sees or messes with the variable that I'm really interested in.

My interest is in the ability to more strongly define who can touch what because I find that to be a common source of bugs.

I can place local functions in nested scopes. (i.e. instead an 'if' block, and whatnot). How would i do that here? Would this literally just be for non-capturing, top-level, local-functions for a method?

No closures. Not really local functions in any sense other than where they can be called from. Just private methods with stronger restrictions for the same reasons we don't make everything public.

Also, how would you handle this for a constructor, destructor, property-getter/setter, or any other places where you could put a local function?

Unless I'm missing something, constructor/finaliser would work fine with the syntax I've already proposed. ClassName.MyMethod can be called from any constructor. ClassName(float, string).MyMethod can only be called from the constructor with those parameters. If the class contains both of those methods, then they must have different parameters like any other overloads, though ClassName(float).MyMethod could potentially be allowed to have the same parameters as ClassName(float, string).MyMethod (I'd lean towards still not allowing it, but syntactically it would work).

Disambiguating between a getter and setter could be done with PropertyName.get.MyMethod or could just be left out. I'm not looking for the ability to narrow the callability down to an exact line number, just something more precise than private.

HaloFour commented 6 years ago

@SilentSin

A method that appears long because it contains a local function definition is not itself long. There's no need for a second syntax to do what we can already do today.

theunrepentantgeek commented 6 years ago

This feature would let you take any large method and split into multiple parts without making all those parts accessible to the rest of the class.

This.

Any time (I'm tempted to say every time) I come across this situation, I break it out into a separate class that performs a specific function.

Sometimes, a private class because it's very specialized; sometimes a public one because it is, well, general purpose and widely useful. Depending on the lifetime, I might make it a struct to avoid allocations.

Creating a separate tightly focused type gives all the encapsulation suggested by this proposal plus the ability to reduce parameter passing by declaring private members for things that don't change during the process.

In Refactoring, Martin Fowler writes about the Large Class code smell (page 78) and says;

More generally, common prefixes of suffixes for some subset of the variables in a class suggest the opportunity for a component.

If a class contained these methods:

void Serialize.WriteHeader(FileStream file) ...
void Serialize.WritePartA(FileStream file) ...
void Serialize.WritePartB(FileStream file) ...
void Serialize.WritePartC(FileStream file) ...
void Serialize.WriteFooter(FileStream file) ...

then I'd suggest Martin's advice would directly apply: the method Serialize is complex enough that it should be broken out into a separate type that can be independently tested.

CyrusNajmabadi commented 6 years ago

It seems to me that you're having trouble seeing the forest through all the trees. Rather than getting caught up in whether or not the intentionally brief and generic examples I've given fit your definition of "too long",

The onus is the person requesting hte feature to provide reasonable examples of how the feature would improve things. If your examples aren't very satisfactory, that's not something i can help with :)

consider any method you have encountered that you would personally put in that category,

I can't imagine any method that would be classified htis way.

I'm sure you've seen some.

I have not :)

consider any situation where you've decided - for whatever reason - to move part of a method out into another private method.

I'm not sure how that's relevant. I make methods private when i want them to be used by other parts of the class, or because the class is so single-purpose that having them be siblings is not a problem.

CyrusNajmabadi commented 6 years ago

What does this feature offer? Even better safety than privates.

But i have htat... with local functions :) You're the one that wants to lift hte local functions up to the class level. I'm saying: i don't see any reason i'd ever want to do that, and it indeed makes the code worse from my perspective (same as if i could place class methods in some random namespace, just 'because').

CyrusNajmabadi commented 6 years ago

Rather than thinking of this feature as "local functions without closures that can be scattered all over the class to create a huge mess", instead think of it as "private methods with additional safety restrictions".

Right... except i already have a solutoin to that. Why do i need another solution. The existing solution works well and does seems to not have any issues (that this language feature would improve). So why have another form of doing the exact same thing (except with lots of complexities and restrictions)?

Defining methods outside the class or out in a namespace probably has merit for some people, but that's the exact opposite of what I'm after.

You're taking a nested symbol and lifting into the parent's container, but stating that is can only be used by the parent. It's the exact same. Why should it be ok to be able to write void FooMethod.LocalFunction() { } in a class, but not the same to do void FooClass.NormalMethod() { } in the namespace containing FooClass?

CyrusNajmabadi commented 6 years ago

Unless I'm missing something, constructor/finaliser would work fine with the syntax I've already proposed. ClassName.MyMethod can be called from any constructor. ClassName(float, string).MyMethod

These seems like an enormously large feature. You have to define an entire way of gramtically specifying the signature for a method, have a way to include that in a class that is not ambiguous with explciit interface impls , have a way to support both explicit interface impls and these in the same signature. Have a way to reference accessors/etc.

All... to allow me to have a method that can be called by one single method... even though i have a way to do that today, which is fairly simple and easy to use, and is a lot more flexible.

Basically, we have an existing feature that does what you need, and has less drawbacks, and doesn't involve inventing a large amount of language features for one very narrow case.

SilentSin commented 6 years ago

@HaloFour At the very least, a method that only appears long because of local methods still needs you to check between and after the local methods to ensure that the outer method isn't doing anything more there and check over each of them in case any of them are using closures.

The inability to create closures with this proposal means that you never have to read through one to double check whether or not it actually is a closure. You call it, it uses the parameters you give and returns a value, done. It's not going to inadvertently modify a local variable in the calling method.

@theunrepentantgeek

Creating a separate tightly focused type gives all the encapsulation suggested by this proposal plus the ability to reduce parameter passing by declaring private members for things that don't change during the process.

Creating a separate type gives none of the encapsulation suggested by this proposal. At best, the separate type can be private and thus not have any more encapsulation than a private method. Not to mention the development overhead of creating more types and managing their relationships.

@CyrusNajmabadi

I can't imagine any method that would be classified htis way [as "too long"].

I don't believe you. Even if you had been able to perfectly utilise separation of concerns from birth, you would have still seen other people's code with single functions longer than you would have done it.

At the very least, you have to acknowledge that other people do classify some methods as too long. It's the second thing under "method level smells" on the Wikipedia page for Code Smells after all.

Right... except i already have a solutoin to that. Why do i need another solution. The existing solution works well and does seems to not have any issues

The jump from private to local methods involves multiple steps. You limit the potential caller (great), and then you limit where the local can be declared (not always so great). I want ReadHeader and WriteHeader to be right next to each other because even though they aren't in the same call chain, any change to one will likely need to be replicated in the other. Those two will need to be read and compared much more often than WriteHeader and WritePartA, and I think that is a reasonable basis for deciding how to organize the code. Locals provide the access control I want, but are too restrictive on the organization of the code.

You're taking a nested symbol and lifting into the parent's container, but stating that is can only be used by the parent. It's the exact same. Why should it be ok to be able to write void FooMethod.LocalFunction() { } in a class, but not the same to do void FooClass.NormalMethod() { } in the namespace containing FooClass?

I'm proposing a middle ground between two existing well justified features and you're comparing it to a middle ground between one existing and one totally new feature (which you seem to agree is a bad idea). There is precedent and justification for each individual part of my proposal already in the language, just not in this particular combination.

These seems like an enormously large feature. You have to define an entire way of gramtically specifying the signature for a method, have a way to include that in a class that is not ambiguous with explciit interface impls , have a way to support both explicit interface impls and these in the same signature. Have a way to reference accessors/etc.

I'd be perfectly happy with ClassName.MyMethod being callable by any overload of MyMethod, with no need for anything more specific. As I said before, I'm not looking to specify everything in exacting detail down to the line number where it can be called, just something safer than privates without going all the way to the restrictions of locals.

All... to allow me to have a method that can be called by one single method... even though i have a way to do that today, which is fairly simple and easy to use, and is a lot more flexible.

Locals: can only be called by one method and must be nested inside that method.

This Proposal: can only be called by one method.

Less restrictions seems like it would mean this proposal is the more flexible one.

CyrusNajmabadi commented 6 years ago

I don't believe you. Even if you had been able to perfectly utilise separation of concerns from birth, you would have still seen other people's code with single functions longer than you would have done it.

Nothing about this suggestion changes that...

The jump from private to local methods involves multiple steps. You limit the potential caller (great), and then you limit where the local can be declared (not always so great). I want ReadHeader and WriteHeader to be right next to each other because even though they aren't in the same call chain, any change to one will likely need to be replicated in the other.

What stops you from putting those right next to each other...?

Less restrictions seems like it would mean this proposal is the more flexible one.

It's a lot less flexible. For example, it can't capture. It can't be scoped.

SilentSin commented 6 years ago

Nothing about this suggestion changes that...

You seem to have missed my point. There are many reasons someone might want to split up a function. A common reason is when a method is getting too large. It's common regardless of whether you personally experience that problem or not.

There are several ways to split a function:

What stops you from putting those right next to each other...?

If they're locals, they have to be inside their outer method. Even if Serialize and Deserialize are next to each other, that still puts a bunch of other locals between WriteHeader and ReadHeader. But it's also quite likely that you have other things to do before serialization and after deserialization, so they'll likely end up even further apart.

If they're privates, nothing stops me. That's what I most commonly do. This proposal would allow me to continue doing that, but with an additional safety measure in place that would help make that approach less bug prone.

It's a lot less flexible. For example, it can't capture. It can't be scoped.

Locals can capture. I don't imagine scoping is particularly useful when you aren't capturing, but that too.

This proposal allows methods to be organized more logically.

They each have different strengths. If you measure flexibility as the frequency with which you encounter situations where a particular approach will be most effective, then I think I'd find this proposal to be much more flexible because I value that organizational ability much more than the ability to capture local variables. And even when I do need to capture, I find it tends to usually involve delegates and thus use good old lambdas instead of local methods.

CyrusNajmabadi commented 6 years ago

Local methods can do it, but are extremely limited in the degree to which you are able to organise them. The entire method is all still in the same area, all you've done is rearrange it a bit. I've already given reasons why that limitation is sometimes undesirable.

Sorry, i either missed, or did not understand the reasoning given for why this was undesirable. Could you clarify that again? Thanks much!

CyrusNajmabadi commented 6 years ago

If they're locals, they have to be inside their outer method. Even if Serialize and Deserialize are next to each other, that still puts a bunch of other locals between WriteHeader and ReadHeader.

Why... i'm not at all understanding why that woudl be the case. Nothing about local functions requires that for example. Can you give a fully fleshed-out example? It would be fine to link to a gist. Right now it's hard to understand what you're talking about because your code snippets don't reflect these statements, and what you are saying does not jive with local functions to me.

Thanks!

CyrusNajmabadi commented 6 years ago

This proposal gives you another option when choosing how to split up a function which avoids some of the problems of each.

Sure. I get that. However, what i was trying to point out was that this is a huge language change. You are defining a huge new syntax (i.e. an entirely new way to reference Methods/Constructors/Accessors/Destructors/etc.). And that huge change you are proposing only serves to let me do something that is only a very very very tiny bit different from waht i can do today already.

That's the antithesis of what i want from a language feature proposal. If there is something huge being suggested, i want to get a massive benefit from it. Or, ideally, i like things that are tiny, but enable huge new capabilities and coding patterns. This proposal feels like the reverse. When i put my LDM hat on, it just makes me feel like i wouldn't ever want to take this if this was all that i got out of it...

HaloFour commented 6 years ago

I'd also suggest that everything that this proposal sets out to achieve could be accomplished via a custom analyzer. You'd write your private methods per normal but adorn them with an attribute that specifies an access control list of which other methods, by name, may invoke it. If the analyzer comes across an invocation of a method with such an attribute from a method that doesn't match the names in the attribute it can return an error diagnostic.

public class C {
    public int Foo() => Secret();
    public int Bar() => Secret();
    public int Baz() => Secret(); // analyzer error here

    [RestrictInvocationTo("Foo", "Bar")]
    private int Secret() => 42;
}
Austin-bryan commented 6 years ago

Yes, in you're simple example, it seems to work, but what about these cases:

public static int BarFoo
{
    get 
    {
        void barFoo() { }
        return 0;
    }
    set 
    {
        void fooBar() { }
    }
}

public static void Main() 
{
    Console.WriteLine("🌄");

    if (true)
    {
        void Foo() { }
    }
    {
        void Bar() { }
    }
    while (false)
    {
        void Baz() { }
    }
    for (int i = 0; i < 10; i++)
    {
        void FooBar() { }
    }
}

Note that you can have multiple ifs, whiles, etc., so how should that be expressed? Main.if1.Foo? That's completely confusing. The reason why local functions are "crammed" into their parent scope is so that they aren't visible where they aren't usable.

What you're asking makes about as much sense as having this syntax:

int Foo.X = 3;    // Only usable in the Foo Method

private void Foo() => X = 2;    // Works
private void Bar() => X = 4    // Doesn't work

If it's not usable by the class, then it shouldn't be in the class body. You're probably wanting some conistency between methods and local methods, but thing is, local methods aren't a thing. These are local functions. If you remember the difference, the difference is that methods belong to a class member, whereas functions don't. Methods are functions, functions aren't methods. It's the same thing with fields and variables. Fields are variables that belong to a class member, whereas variables aren't. C# has local variables, but not local fields, that doesn't make sense.

Local functions are the counterpart to local variables and it would be confusing to have them in class level scope like it would be to have local variables in class level scope.

SilentSin commented 6 years ago

@CyrusNajmabadi

Sorry, i either missed, or did not understand the reasoning given for why this was undesirable. Could you clarify that again? Thanks much!

Let's stick with the serialize/deserialize example, where each contains a bunch of calls to write/read specific sections of a file. This could be any pair or larger set of methods such as Initialise/Dispose, Enable/Disable, EnterState/ExitState, etc. Broadly speaking, there are two ways you could lay out those concepts in code.

  1. You can group Serialize with all the Writes and Deserialize with all the Reads, either as local functions or just privates near each other. Or even just have Serialize and Deserialize as large single functions.
  2. Or you can group Serialize with Deserialize, WriteHeader with ReadHeader, and so on. This can't be done with locals and you would only gain additional complexity by adding a Header class, without any benefit to encapsulation. So it is best done with privates.

Approach 1 lets you most easily go to Serialize to see and compare what each of the Writes is doing. But, there is no direct connection between the Read and Write methods. If you make a change to WriteHeader, there is nothing telling you that ReadHeader will need a similar change. If you want to compare the two side by side you would need to manually open a second window and navigate to the other method, possibly by using Find All References on one of the fields it touches and going through them to find the one you want.

Approach 2 lets you most easily see that the methods are paired and compare what they are doing, which is useful because a bug that appears in one could quite likely have been caused by a bug in the other. But, to go through Serialize and see what it's doing like before, you would need to use IDE features like Go To Definition and Find All References to go back and forth between the caller and each of the calls.

Approach 1 does it's good part well but has poor support for other aspects while Approach 2 still does its good part well but has much better IDE support for its deficiencies. Even without an IDE, Approach 2 still has a clear link between the caller and the methods it calls where there is no such link between the Reads and Writes otherwise.

Why... i'm not at all understanding why that woudl be the case. Nothing about local functions requires that for example.

What do you mean "nothing about local functions requires that"? Local functions must be inside their method. If Serialize has 5 Write methods and Deserialize has 5 Read methods, only the bottom local from one and the top from the other can actually be close to each other. The rest will have multiple functions between them.

Sure. I get that. However, what i was trying to point out was that this is a huge language change. You are defining a huge new syntax (i.e. an entirely new way to reference Methods/Constructors/Accessors/Destructors/etc.). And that huge change you are proposing only serves to let me do something that is only a very very very tiny bit different from waht i can do today already.

I don't claim to know anything about the internal workings of compilers or the effort that would be required to implement such a feature.

All I can see is that syntactically - in terms of how easy this feature would be for someone to figure out when learning the language - it seems very straightforward and quite similar to explicit interface methods.

That's the antithesis of what i want from a language feature proposal. If there is something huge being suggested, i want to get a massive benefit from it. Or, ideally, i like things that are tiny, but enable huge new capabilities and coding patterns. This proposal feels like the reverse. When i put my LDM hat on, it just makes me feel like i wouldn't ever want to take this if this was all that i got out of it...

That's a valid point, but it seems to be that you only feel the benefits are insignificant because you personally don't use the second organisational approach I described above rather than acknowledging that there are many different ways to do things. Local methods see very little use in my approach, but I don't try to pretend they can't be useful for other people.

@HaloFour

I'd also suggest that everything that this proposal sets out to achieve could be accomplished via a custom analyzer.

That does sound like it would mostly do the trick. Not quite as elegantly and you'd have to use nameof for everything so you don't have to manually fix stuff when refactoring, but you'd also get a bit more flexibility (like allowing two methods as you've shown).

On a somewhat related note, but without wanting to derail this thread: do you think a proposal for a more compact nameof expression would have any hope? For example: @Foo instead of nameof(Foo). I can't immediately think of any ways in which that would conflict with the existing use of the @ symbol.

Edit: it would conflict when using @keyword for a variable name. Maybe another symbol like $?

@willard720 I'm not sure which part of the conversation you think you're responding to, but I've stated several times (including in the OP) that I'm not proposing these methods have anything to do with closures or scoping within the target method at all. These are methods that exist outside the target method, so there is no need to try to define syntax to determine which block might contain them inside that method.

You're probably wanting some consistency between methods and local methods

Consistency between them? No, they're both separate and serve their own purposes. But that doesn't mean that there can't be a useful middle ground between them as well.

but thing is, local methods aren't a thing. These are local functions.

Local functions weren't a thing either ... until they were implemented.

We don't have local fields right now, but a proposal such as https://github.com/dotnet/csharplang/issues/832 could easily fall under that name and wouldn't simply be refused because the current semantics we use to talk about the language don't have them. Yeah we don't have local fields, but if you called a local variable a local field by accident the other person would still know exactly what you're talking about. I'm not here to play semantic word games.

CyrusNajmabadi commented 6 years ago

This can't be done with locals and you would only gain additional complexity by adding a Header class, without any benefit to encapsulation. So it is best done with privates.

Sorry, i'm still not understanding :). You say it would be complexity to add a header class (or, as i would prefer here, a struct), but i don't get why that's defacto so. Classes/structs do a great job of providing this sort of encapsulation. So much so, there's even a refactoring pattern named for this: "refactoring method with method object".

This works well, and gives you the most flexibility, as you now have the power of everything a class/struct can do at your disposal. You have full control over scoping, over variables, over your methods (which are effectively the local functions), which can then themselves have local-functions (or even other method-objects themselves).

You have maximal ability for organization. You can place your code wherever you feel is best. You don't have limited restrictions like the ones in your proposal.

And, best of all, this can all be done with the facilities in the language today :)

it feels much more complex to add this new language feature. First, you now have another way to say something that's already possible with existing constructs. Second, you have to come up with an entire way to describe member-references (syntactically and semantically). Third, that new construct you have would be very limited (for example, i could not state that i wanted a local function that was only available inside one branch of an if-statement). etc. etc.

You say using the existing language features would be more complex, but it sounds orders of magnitude less complex than going this other route :)

CyrusNajmabadi commented 6 years ago

I don't claim to know anything about the internal workings of compilers or the effort that would be required to implement such a feature.

All I can see is that syntactically - in terms of how easy this feature would be for someone to figure out when learning the language - it seems very straightforward and quite similar to explicit interface methods.

Explicit-interface methods have no concept of what you're referring to here. There is legitimately nothing in the grammar of the language to support what you're trying to do. You'd have to invent an entire way of specifying method-references, constructor-references, destructor-references, operator-references, accessor-references, etc. etc.

Just explore that for a bit. How would you say you wanted to define a local function that would only be available in one your overloaded operators? Or in one of your conversion operators.

--

Note: i'm not even referring to compiler complexity. I'm just referring to language complexity. You'd have to define a large amount of syntactic and semantic rules, and all that would let you do is something you can already effectively do (and do more of) with other existing constructs that haven't required that sort of complexity :)

SilentSin commented 6 years ago

Sorry, i'm still not understanding :). You say it would be complexity to add a header class (or, as i would prefer here, a struct), but i don't get why that's defacto so. Classes/structs do a great job of providing this sort of encapsulation. So much so, there's even a refactoring pattern named for this: "refactoring method with method object".

I have a class with some fields of existing types, some methods for serialization, and some methods for deserialization.

You have a class with nested classes/structs for each part, a field of each of those types in your outer class, extra operations in your properties to expose the same fields, extra stuff you need to do for initialisation, and so on. That's a lot more complexity added where this proposal would achieve the same thing with a simple prefix to the method name.

Edit: actually, this proposal wouldn't only achieve the same thing, it would still be better (in this specific aspect, not necessarily in all areas) because ReadHeader would only be accessible in one place where a separate class still leaves it accessible to everything in the outer class. And if the inner classes have references to the outer to share some of its stuff between them, then it goes right back to being accessible to all of them too.

it feels much more complex to add this new language feature.

Of course adding a new feature is going to be more complex than not adding it. The point is that once added using it is just writing a prefix vs. creating a nested class, adding a field for it, moving all your fields around, giving it a reference to the outer class if there's anything it shares with the other parts, etc.

Second, you have to come up with an entire way to describe member-references (syntactically and semantically).

It's not coming up with an entire new way, it's adapting an existing way.

void IDisposable.Dispose() { } can only be called on an IDisposable.

Action myAction = Serialize; is already a way of referencing a method.

void Serialize.WriteHeader() { } can only be called from inside Serialize.

Conceptually that seems extremely straightforward to me.

Third, that new construct you have would be very limited (for example, i could not state that i wanted a local function that was only available inside one branch of an if-statement). etc. etc.

I've already said I don't see a need for anything that specific, but if you did it could potentially target a goto token (or whatever they're called). Serialize.OhNoesAnException.Fuuuuuck() { } can only be called after the OhNoesAnException: (and within the same scope). That wouldn't guarantee a unique token name, but it would be more restrictive than just "any method with this name". I suspect that adding this aspect might actually require more effort than the whole rest of the proposal because it would need those tokens to be visible outside the method (only in a very specific context, but visible all the same).

I don't think that a feature which can apply to any private method that is only intended to be called in one place is particularly limited. Maybe it's just because I'm not a fan of nesting methods inside methods inside methods, which is what I'd have to do to move them to locals. And also that I don't like the idea of needing to check for closures all the time even when they aren't being used.

Explicit-interface methods have no concept of what you're referring to here. There is legitimately nothing in the grammar of the language to support what you're trying to do. You'd have to invent an entire way of specifying method-references, constructor-references, destructor-references, operator-references, accessor-references, etc. etc.

You say that as if they're all vastly different things that would be difficult to understand. Difficult to implement, sure, but how hard do you really think it would be to understand this:

Just explore that for a bit. How would you say you wanted to define a local function that would only be available in one your overloaded operators? Or in one of your conversion operators.

Overloaded operators I just covered.

Conversion operators would fall in line with my opinion that targeting a specific scope within a method or even a specific overload of a method isn't strictly necessary. So targeting the ClassName would get you into conversion operators as well as constructors. And since conversion operators are conceptually quite similar to extra constructor overloads, I think that would be fine.

If the feature does absolutely need to target only one method overload (or have optional syntax for specifying that), then operator.ClassName(param types).TargetedMethodName could cover it in a similar manner as would be needed for specifically targeting the getter or setter of a property. But in that case I agree with you that this would be adding completely new syntax and places where keywords can appear which - while generally straightforward - aren't really similar to anything we currently have.

Note: i'm not even referring to compiler complexity. I'm just referring to language complexity. You'd have to define a large amount of syntactic and semantic rules, and all that would let you do is something you can already effectively do (and do more of) with other existing constructs that haven't required that sort of complexity :)

As with compiler complexity, I know very little about the underlying language complexity and degree to which things need to be explained in the language spec. I don't care whether X is a field or property or type or whatever, I just write X.Y to access whatever Y happens to be in relation to X. Case closed. Make it happen computer. I'm being a bit sarcastic of course, but my point is that from a high-level end-user point of view (my point of view) this feature would not be so complex to learn or use in practice.

Perhaps it would have been helpful if you had been a little clearer (or maybe I just missed it) that your complaints about the complexity are focussed on the development of the feature rather than on the actual usage (or learning). You're complaining about a massive ice berg from below while it looks tiny to me on the surface. :)

gafter commented 6 years ago

Closing as dup of #1329