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.

Sign up for GitHub

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 proposal for BinaryCompatOnlyAttribute #7707

Closed
wants to merge 1 commit into from
Closed

Add proposal for BinaryCompatOnlyAttribute #7707

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 123 additions & 0 deletions proposals/binary-compat-only.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
# BinaryCompatOnlyAttribute

## Summary
[summary]: #summary

We introduce a new attribute, `System.BinaryCompatOnlyAttribute`, that causes the type or member to which it is applied to be entirely inaccessible from source code.
Copy link
Member

Choose a reason for hiding this comment

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

Is it inaccessible from within the current compilation or just referencing compilations?

It will still be emitted as declared, with the accessibility declared, but will be treated as if it does not exist by all other compile-time C# mechanisms.
Copy link
Member

Choose a reason for hiding this comment

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

so can a method not call itself?

what if i have multiple BCOA members, where one calls the other. i feel like that would be totally normal.


## Motivation
[motivation]: #motivation

API authors often run into an issue of what to do with a member after it has been obsoleted. For backwards compatibility purposes, many will keep the existing member around
with `ObsoleteAttribute` set to error in perpetuity, in order to avoid breaking consumers who upgrade binaries at runtime. This particularly hits plugin systems, where the
author of a plugin does not control the environment in which the plugin runs. The creator of the environment may want to keep an older method present, but block access to it
for any newly developed code. However, `ObsoleteAttribute` by itself is not enough. The type or member is still visible in overload resolution, and may cause unwanted overload
resolution failures when there is a perfectly good alternative, but that alternative is either ambiguous with the obsoleted member, or the presence of the obsoleted member causes
overload resolution to end early without ever considering the good member. For this purpose, we want to have a way to mark such members as technically being present in the DLL,
but not visible to any code, so that they will have no impact on member lookup, overload resolution, or any other similar elements.
Copy link
Member

Choose a reason for hiding this comment

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

Another motivation is that lacking a feature like [BinaryCompatOnly] we are limited in our ability to add new caller info style attributes. These two issues have good discussions on this: caller identity and xunit


## Detailed design
[design]: #detailed-design

### `System.BinaryCompatOnlyAttribute`

We introduce a new reserved attribute:

```cs
namespace System;

// Excludes Assembly, GenericParameter, Module, Parameter, ReturnValue
[AttributeUsage(AttributeTargets.Class
| AttributeTargets.Constructor
| AttributeTargets.Delegate
| AttributeTargets.Enum
| AttributeTargets.Event
| AttributeTargets.Field
| AttributeTargets.Interface
| AttributeTargets.Method
| AttributeTargets.Property
| AttributeTargets.Struct,
AllowMultiple = false,
Inherited = false)]
public class BinaryCompatOnlyAttribute : Attribute {}
Copy link

Choose a reason for hiding this comment

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

I presume this should be sealed.

```

When applied to a type member, that member is treated as inaccessible in every location by the compiler, meaning that it does not contribute to member
Copy link
Member

Choose a reason for hiding this comment

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

Can a BinaryCompatOnly member reference another BinaryCompatOnly member?
e.g,

[BinaryCompatOnly]
public class C1 { }

[BinaryCompatOnly]
public class C2 : C1 { }

Copy link

Choose a reason for hiding this comment

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

I think that should be possible, just like it is with [Obsolete] elements referring to other [Obsolete] elements

Copy link
Member Author

@333fred 333fred Dec 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

As currently proposed, no. This is one of several open questions that I get into.

Copy link

Choose a reason for hiding this comment

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

I think this shouldn't be possible, you can always extract a binary compat member's impl to another private / internal member as required, and then just keep the binary compat member as a stub to it. Since the member would not be exposed externally, you can just name it whatever works; this allows effectively calling the compat only member, without having to complicate overload rules and similar to figure out which one of the potentially conflicting overloads you wanted (if we allow overloading by ref/virtual in, return types, etc with this feature, which I assume we're allowing, as long as they're binary different).

lookup, overload resolution, or any other similar process.

### Accessibility Domains

We update [§7.5.3 Accessibility domains](https://github.com/dotnet/csharpstandard/blob/720d921c5688190ea544682cdbdf8874fa716f2b/standard/basic-concepts.md#753-accessibility-domains)
as **follows**:

> The ***accessibility domain*** of a member consists of the (possibly disjoint) sections of program text in which access to the member is permitted. For purposes of defining the accessibility domain of a member, a member is said to be ***top-level*** if it is not declared within a type, and a member is said to be ***nested*** if it is declared within another type. Furthermore, the ***program text*** of a program is defined as all text contained in all compilation units of the program, and the program text of a type is defined as all text contained in the *type_declaration*s of that type (including, possibly, types that are nested within the type).
>
> The accessibility domain of a predefined type (such as `object`, `int`, or `double`) is unlimited.
>
> The accessibility domain of a top-level unbound type `T` ([§8.4.4](types.md#844-bound-and-unbound-types)) that is declared in a program `P` is defined as follows:
>
> - **If `T` is marked with `BinaryCompatOnlyAttribute`, the accessibility domain of `T` is completely inaccessible to the program text of `P` and any program that references `P`.**
> - If the declared accessibility of `T` is public, the accessibility domain of `T` is the program text of `P` and any program that references `P`.
> - If the declared accessibility of `T` is internal, the accessibility domain of `T` is the program text of `P`.
>
> *Note*: From these definitions, it follows that the accessibility domain of a top-level unbound type is always at least the program text of the program in which that type is declared. *end note*
>
> The accessibility domain for a constructed type `T<A₁, ..., Aₑ>` is the intersection of the accessibility domain of the unbound generic type `T` and the accessibility domains of the type arguments `A₁, ..., Aₑ`.
>
> The accessibility domain of a nested member `M` declared in a type `T` within a program `P`, is defined as follows (noting that `M` itself might possibly be a type):
>
> - **If `M` is marked with `BinaryCompatOnlyAttribute`, the accessibility domain of `M` is completely inaccessible to the program text of `P` and any program that references `P`.**
> - If the declared accessibility of `M` is `public`, the accessibility domain of `M` is the accessibility domain of `T`.
> - If the declared accessibility of `M` is `protected internal`, let `D` be the union of the program text of `P` and the program text of any type derived from `T`, which is declared outside `P`. The accessibility domain of `M` is the intersection of the accessibility domain of `T` with `D`.
> - If the declared accessibility of `M` is `private protected`, let `D` be the intersection of the program text of `P` and the program text of `T` and any type derived from `T`. The accessibility domain of `M` is the intersection of the accessibility domain of `T` with `D`.
> - If the declared accessibility of `M` is `protected`, let `D` be the union of the program text of `T`and the program text of any type derived from `T`. The accessibility domain of `M` is the intersection of the accessibility domain of `T` with `D`.
> - If the declared accessibility of `M` is `internal`, the accessibility domain of `M` is the intersection of the accessibility domain of `T` with the program text of `P`.
> - If the declared accessibility of `M` is `private`, the accessibility domain of `M` is the program text of `T`.

The goal of these additions is to make it so that members marked with `BinaryCompatOnlyAttribute` are completely inaccessible to any location, they will
not participate in member lookup, and cannot affect the rest of the program. Consequentely, this means they cannot implement interface members, they cannot
Copy link
Member

Choose a reason for hiding this comment

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

Consequentely, this means they cannot implement interface members

this also seems wrong. Say i have a BCOA interface member, and then the BCOA impl of htat interface member. That seems no longer supported.

call each other, and they cannot be overridden (virtual methods), hidden, or implemented (interface members). Whether this is too strict is the subject of
several open questions below.
Copy link
Member

Choose a reason for hiding this comment

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

The interface member part will lead to some interesting situations. Consider the following as a concret example:

// assembly1:
public abstract class C {
  public void Dispose() { }
}

// assembly2:
public class D : C, IDisposable { }

Now lets say in version 2 the author of assembly1 marks C.Dispose with [BinaryCompatOnly]. Execution of assembly2 will continue to work fine because C.Dispose still satisfies IDisposable.Dispose by CLR rules. The moment the author moves to a new version of assembly1 they will get a compilation error and be forced to re-implement IDisposable.Dispose. That overall may be okay but it seems like a potential pitfall in some cases.

Copy link

Choose a reason for hiding this comment

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

I mentioned a potential way to solve the usability concerns around this section in my comments below.

I don't understand why hiding would be disallowed here - doesn't shadowing have nothing to do with the original member really? Especially since BinaryCompatOnly effecively pre-emptively shadows a member before anyone can even use it, so I don't see why it would be disallowed here.


## Drawbacks
[drawbacks]: #drawbacks

More complex attribute mechanisms make the language more complex.

## Alternatives
[alternatives]: #alternatives

No other alternatives have presented themselves

## Unresolved questions
[unresolved]: #unresolved-questions

### Virtual methods and overriding

What do we do when a virtual method is marked as `BinaryCompatOnly`? Overrides in a derived class may not even be in the current assembly, and it could
be that the user is looking to introduce a new version of a method that, for example, only differs by return type, something that C# does not normally
allow overloading on. What happens to any overrides of that previous method on recompile? Are they allowed to override the `BinaryCompatOnly` member if
they're also marked as `BinaryCompatOnly`?

### Use within the same DLL

This proposal states that `BinaryCompatOnly` members are not visible anywhere, not even in the assembly currently being compiled. Is that too strict, or
do `BinaryCompatAttribute` members need to possibly chain to one another?
Copy link
Member

Choose a reason for hiding this comment

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

How does this work with say unit testing? The primary consumer of this technology is likely to be the .NET libraries / runtime team. The [BinaryCompatOnly] attribute will make the method inaccessible to new code but old code will still use it. The method will exist virtually forever for this reason.

How will the libraries team write unit tests for this method that they need to maintain forever?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe TestUtils.CreateDelegate<Action<DeclaringType, TParam1, ...>>("MethodName") and similar strategies for invoking the method using reflection with low ceremony at the call site?

Copy link

Choose a reason for hiding this comment

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

I think a good solution to this is the following: disallow referencing BinaryCompatOnly members (other than types) completely, their implementation can be extracted to a private / internal member as required which the BinaryCompatOnly will stub to. For unit tests, you would either test the stub, or use reflection.

There is a more interesting question with types though, as you still need to be able to use them in signatures probably (even if just other BinaryCompatOnly signatures); I think the solution to this could be: only use that type if there's no other type which matches (i.e., force all BinaryCompatOnly types to match very last, after every other possibility) (or can be done by specifying full name global:: if needed - probably the recommended way to do it), and emit a warning on usage of a BinaryCompatOnly type (default error severity), which can be suppressed if the usage is intentional. Potential (quite reasonable) adjustment could be that: this only applies for already BinaryCompatOnly members/types, it would be disallowed & ignored otherwise.


### Implicitly implementing interface members

Should `BinaryCompatOnly` members be able to implement interface members? Or should they be prevented from doing so. This would require that, when a user
wants to turn an implicit interface implementation into `BinaryCompatOnly`, they would additionally have to provide an explicit interface implementation,
likely cloning the same body as the `BinaryCompatOnly` member as the explicit interface implementation would not be able to see the original member anymore.
Copy link
Member

Choose a reason for hiding this comment

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

The interface member part will lead to some interesting situations. Consider the following as a concret example:

// assembly1:
public abstract class C {
  public void Dispose() { }
}

// assembly2:
public class D : C, IDisposable { }

Now lets say in version 2 the author of assembly1 marks C.Dispose with [BinaryCompatOnly]. Execution of assembly2 will continue to work fine because C.Dispose still satisfies IDisposable.Dispose by CLR rules. The moment the author moves to a new version of assembly1 they will get a compilation error and be forced to re-implement IDisposable.Dispose. That overall may be okay but it seems like a potential pitfall in some cases.

Copy link

Choose a reason for hiding this comment

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

Perhaps we could have something like BinaryCompatOnlyImplementation(Type), where it specifies an interface which should only be considered to be implemented for binary compatibility purposes. This would allow removing interfaces implementations from usage in source, without breaking their usage in binary.

With this setup, you would disallow explicit interface implementations from being marked BinaryCompatOnly, unless their definition is also this as I proposed in my other comment; for a non-BinaryCompatOnly definition, if the implicit one is marked BinaryCompatOnly, then an explicit one would have to be provided.


### Implementing interface members marked `BinaryCompatOnly`

What do we do when an interface member has been marked as `BinaryCompatOnly`? The type still needs to provide an implementation for that member; it may be
that we must simply say that interface members cannot be marked as `BinaryCompatOnly`.
Copy link

Choose a reason for hiding this comment

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

For this, I think we could allow it like so: if an interface member definition is marked BinaryCompatOnly, then its implementation must also be explicit, and marked BinaryCompatOnly. All of the parameters & return type would have to exactly match also.


## Design meetings

Link to design notes that affect this proposal, and describe in one sentence for each what changes they led to.