Repository navigation
Proposal: Hide Thread.VolatileRead and Thread.VolatileWrite #27997
Description
Rationale
Thread.VolatileRead and Thread.VolatileWrite delegate to Volatile.Read and Volatile.Write, but it is an incomplete delegation in that Volatile has support for bool whereas Thread does not.
I always reach for Thread.VolatileRead before Volatile.Read. Hiding it would help break this habit.
Proposal
namespace System.Threading
{
public sealed class Thread : CriticalFinalizerObject
{
...
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static byte VolatileRead(ref byte address) => Volatile.Read(ref address);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static double VolatileRead(ref double address) => Volatile.Read(ref address);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static short VolatileRead(ref short address) => Volatile.Read(ref address);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static int VolatileRead(ref int address) => Volatile.Read(ref address);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static long VolatileRead(ref long address) => Volatile.Read(ref address);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static IntPtr VolatileRead(ref IntPtr address) => Volatile.Read(ref address);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static object VolatileRead(ref object address) => Volatile.Read(ref address);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static sbyte VolatileRead(ref sbyte address) => Volatile.Read(ref address);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static float VolatileRead(ref float address) => Volatile.Read(ref address);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static ushort VolatileRead(ref ushort address) => Volatile.Read(ref address);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static uint VolatileRead(ref uint address) => Volatile.Read(ref address);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static ulong VolatileRead(ref ulong address) => Volatile.Read(ref address);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static UIntPtr VolatileRead(ref UIntPtr address) => Volatile.Read(ref address);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref byte address, byte value) => Volatile.Write(ref address, value);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref double address, double value) => Volatile.Write(ref address, value);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref short address, short value) => Volatile.Write(ref address, value);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref int address, int value) => Volatile.Write(ref address, value);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref long address, long value) => Volatile.Write(ref address, value);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref IntPtr address, IntPtr value) => Volatile.Write(ref address, value);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref object address, object value) => Volatile.Write(ref address, value);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref sbyte address, sbyte value) => Volatile.Write(ref address, value);
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref float address, float value) => Volatile.Write(ref address, value);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref ushort address, ushort value) => Volatile.Write(ref address, value);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref uint address, uint value) => Volatile.Write(ref address, value);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref ulong address, ulong value) => Volatile.Write(ref address, value);
[CLSCompliant(false)]
+ [EditorBrowsable(EditorBrowsableState.Never)]
public static void VolatileWrite(ref UIntPtr address, UIntPtr value) => Volatile.Write(ref address, value);
...
}
}
Activity
carlreinke commentedon Dec 7, 2018
cc: @kouvel
carlreinke commentedon Feb 5, 2019
Is there something I need to do to get someone to take a look at this? It ought to be a simple thumbs up or down.
danmoseley commentedon Feb 5, 2019
Cc @kouvel
This does not seem a compelling reason on the face of it. It can cause more confusion eg if you are used to using these methods and no longer see them in Intellisense. I doubt we would change this. You might consider a Roslyn analyzer or simple grep script.
carlreinke commentedon Feb 5, 2019
Would a proposal to add
Thread.VolatileReadandThread.VolatileWriteoverloads forboolbe more acceptable?If a developer discovers
Thread.VolatileRead/Thread.VolatileWritebefore theVolatileclass then they're going to conclude that volatile operations can't be performed onbool, as was the case for a long time in Framework.kouvel commentedon Feb 5, 2019
I would prefer to deprecate the
Threadversions over adding new overloads there. They are effectively deprecated APIs. @stephentoub, what is the proper way to deprecate APIs in .NET Core? Hiding them in the editor seems reasonable to me since we would like people to switch over to theVolatileversions.stephentoub commentedon Feb 7, 2019
We generally don't Obsolete things; there are a variety of problems with that.
@terrajobst has been exploring an alternate analyzer-based mechanism; he can provide more details.
carlreinke commentedon Mar 12, 2019
Can this be ready-for-review or does something else need to happen first?
terrajobst commentedon Nov 12, 2019
Video
sharwell commentedon Nov 12, 2019
I would love to see a volatile read with
inparameters. I don't think eitherThreadorVolatileoffers these today.stephentoub commentedon Nov 12, 2019
I don't see the benefit of hiding these. Why is it valuable, @terrajobst?
sharwell commentedon Nov 12, 2019
I also lean against hiding these. It would be preferable to allow someone to reference them, followed by a diagnostic analyzer reporting the alternative and providing a code fix for it.
danmoseley commentedon Nov 12, 2019
Why not just add the bool overloads? Is it a simple oversight that there are all the others?
paulomorgado commentedon Jan 14, 2020
@sharwell, I would mix both. The obsolete attribute could have an overload with the replacement API and a generic analyzer/fixer for this attribute would offer the proper fix.
23 remaining items
Youssef1313 commentedon Feb 20, 2021
Per #27997 (comment), this was approved to be marked as Obsolete. However, the labels says "code-analyzer" and "code-fixer". Are the labels incorrect?
carlreinke commentedon Feb 20, 2021
@Youssef1313 The approval is contingent on the implementation of an analyzer and fixer.
stephentoub commentedon Feb 21, 2021
I expect we'd just need a fixer for whatever diagnostic ID is included as part of the obsoletion attribute.
CollinAlpert commentedon Nov 30, 2023
I have implemented the fixer and now need to settle on a diagnostic ID. Can I confirm the ID "SYSLIB0054" for this obsoletion?