Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API proposal: obsolete DisablePrivateReflectionAttribute #43247

Closed
GrabYourPitchforks opened this issue Oct 10, 2020 · 1 comment · Fixed by #49550
Closed

API proposal: obsolete DisablePrivateReflectionAttribute #43247

GrabYourPitchforks opened this issue Oct 10, 2020 · 1 comment · Fixed by #49550
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.CompilerServices help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@GrabYourPitchforks
Copy link
Member

Per #11811, the runtime does not honor DisablePrivateReflectionAttribute, and there are no plans to restore this capability. We should obsolete the API.

namespace System.Runtime.CompilerServices
{
    [Obsolete("DisablePrivateReflectionAttribute has no effect in .NET Core and .NET 5.0+ applications.")] // NEW attribute
    [AttributeUsage(AttributeTargets.Assembly, AllowMultiple = false, Inherited = false)]
    public sealed class DisablePrivateReflectionAttribute : Attribute
    {
        public DisablePrivateReflectionAttribute() { }
    }
}

I recommend using a normal [Obsolete] attribute with no diagnostic id since there's no real required reading here, nor should people really be suppressing this. They should just remove the attribute. There is no replacement mechanism to restore this functionality.

If we really wanted a diagnostic id and the whole breaking change rigamarole that it entails, we could use SYSLIB0013. I'll file a separate doc issue regardless of whether we use a unique diagnostic id here.

@GrabYourPitchforks GrabYourPitchforks added area-System.Runtime.CompilerServices api-ready-for-review API is ready for review, it is NOT ready for implementation labels Oct 10, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Oct 10, 2020
@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Oct 20, 2020
@terrajobst
Copy link
Member

terrajobst commented Oct 20, 2020

Video

  • We should use a diagnostic ID. This allows us to have a help-topic specific for this issue. SYSLIB0013 sounds yes. The topic seems to warrant that.
  • Other than that, looks good.
namespace System.Runtime.CompilerServices
{
    [Obsolete("DisablePrivateReflectionAttribute has no effect in .NET Core and .NET 5.0+ applications.")]
    public partial class DisablePrivateReflectionAttribute
    {
    }
}

@krwq krwq added this to Krzysztof's triage backlog in Triage POD for Meta, Reflection, etc Feb 4, 2021
@krwq krwq moved this from Krzysztof's triage backlog to Needs triage in Triage POD for Meta, Reflection, etc Feb 4, 2021
@joperezr joperezr added this to the Future milestone Feb 16, 2021
@joperezr joperezr removed the untriaged New issue has not been triaged by the area owner label Feb 16, 2021
@joperezr joperezr moved this from Needs triage to Future in Triage POD for Meta, Reflection, etc Feb 16, 2021
@joperezr joperezr added the help wanted [up-for-grabs] Good issue for external contributors label Feb 16, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 12, 2021
Triage POD for Meta, Reflection, etc automation moved this from Future to Done Mar 17, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.CompilerServices help wanted [up-for-grabs] Good issue for external contributors
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants