Compiler generated code handling

[vitek-karas]

Also adds tests for the existing patterns - all of them fail. Either lack of implementation or lack of known outcome. The tests are there as they are the complete examples (not just the short versions in the doc).

This is meant to start the discussion on #2001 - determine desired behaviors and eventually also about possible solutions.

[marek-safar]

These are all C# specific examples, maybe you could group them that way.

[vitek-karas]

I tried to add text which describes that this should ideally be for all compilers, but that we're focusing on C# right now.

[marek-safar]

Which languages suffer from this problem?

[vitek-karas]

C# - is the one I know of. VB probably as well (didn't try).
While I agree that we should try to come up with solutions which could work for all languages, I think it's OK to start by focusing on C# first.

[marek-safar]

I don't have a problem focusing on C# but the wording used in the design doc should indicate that more clearly.

[MichalStrehovsky]

Thank you for writing these down!

I think another interesting aspect is how we handle lack of annotations.

E.g. would we want to tell the user that for this:

static void TestLambdaWithCapture(int p)
{
    Action a = () => MethodRequiresUnreferencedCode(p);
}

the problem is in the TestLambdaWithCapture method and not with the compiler-generated closure class?

[eerhardt]

Open question Q1a: Should method body attributes propagate to lambdas?
Question Q1b: Should method body attributes propagate to local functions?

(My opinion) To a developer these look like different methods. Assuming the C# 10 feature gets in for lambda method attributes, I don't feel it would be necessary to propagate the attributes to lambdas and local functions. But obviously for iterators and async methods, these don't appear to be different methods to a developer. Which is why the attributes applying to the "whole method" (from a dev's point of view) makes sense there.

[vitek-karas]

@MichalStrehovsky I added a new section about method names in warnings. It's a very good point!
That said I think we've got it already covered for the most part:

• If there are symbols we already point to the correct source line. There are some tweaks we should do, but in the end the warning should look almost identical to what the analyzer produces - meaning it doesn't include the name of the method at all - it solely relies on the source line info for that.
• If there are no symbols, we show the IL method name currently. We can discuss if it's worth trying to improve on this - but it should be a much less important case, since we expect most warnings to have symbols (and those which don't should be collapsed).

[vitek-karas]

@eerhardt I'm yet to write down recommended solution and such... but to comment on your recommendation that lambdas and local functions should not need automatic propagation because developers can explicitly annotate them:

In general I agree, but it might be pretty tricky to achieve. Closure+Lambdas and iterators look very similar at the IL level - we would need some new information from the compiler to be able to differentiate (as far as I can tell). But that's an implementation detail. More importantly SuppressMessageAttribute and UnconditionalSuppressMessageAttribute in Roslyn already applies to lambdas and local functions - it effectively propagates (I assume the latter was modeled by the former in that behavior). So linker should (basically must) implement the same behavior - unless we want to change the behavior in Roslyn.

We could still do something different for RUC, but I'm leaning towards keeping it simple (and thus consistent).

[MichalStrehovsky]

@MichalStrehovsky I added a new section about method names in warnings

There's also a question about dataflow annotations:

static IEnumerable<int> TestParameter(Type type)
{
    type.GetMethod("BeforeIteratorMethod");
    yield return 1;
    type.GetMethod("AfterIteratorMethod");
}

(I took your example and removed the annotation on Type.)

In the ideal world, this would produce a warning saying that parameter type doesn't have a PublicMethods annotation (but the way to get to that warning is... complex...).

[agocke]

One interesting thing to note: if the code generated ends up in types, we could unambiguously refer to the type via typeof. But if the user code ends up in a bare method, there's no equivalent encoding for method signatures, including custom modifiers.

[agocke]

I checked and it looks like async and iterator methods each get an attribute, Async/IteratorStateMachine along with a typeof pointing to the definition. We could consider generalizing that in some way.

[agocke]

@marek-safar Do you happen to have a design doc for VB or F# for the other linker features? We recently discovered serious deficiencies with VB, so I wonder if the rest of the linker was designed with other languages in mind.

[marek-safar]

@marek-safar Do you happen to have a design doc for VB or F# for the other linker features?

In pre-netcore version of linker we didn't add any language-specific features. There were added later. In our previous testing, we generally focused on C# and F# only.

[vitek-karas]

I added first draft for recommended solutions. Currently it's very high-level.

[vitek-karas]

I'm going to merge this so that we have at least some documentation in the repo around this.
Part of this was already implemented in .NET 6. The .NET 7 part is being worked on.