Skip to content

Commit

Permalink
Remove stale warning 420 pragmas (dotnet/corefx#35023)
Browse files Browse the repository at this point in the history
It used to be that the CS0420 warning ("a reference to a volatile field will not be treated as volatile") would fire when a volatile was used with an Interlocked.* operation.  That warning was unnecessary, as Interlocked.* would itself provide the relevant barriers, and these functions were special cased in Roslyn a long time ago.  But there are still lots of places where pragmas disabling the warning have stuck around.  I'm deleting them.

Commit migrated from dotnet/corefx@8b3446f
  • Loading branch information
stephentoub authored Feb 1, 2019
1 parent 126a59e commit 860ccad
Show file tree
Hide file tree
Showing 24 changed files with 9 additions and 101 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,8 @@ private bool TryAddWithNoTimeValidation(T item, int millisecondsTimeout, Cancell
while (_currentAdders != COMPLETE_ADDING_ON_MASK) spinner.SpinOnce();
throw new InvalidOperationException(SR.BlockingCollection_Completed);
}
#pragma warning disable 0420 // No warning for Interlocked.xxx if compiled with new managed compiler (Roslyn)

if (Interlocked.CompareExchange(ref _currentAdders, observedAdders + 1, observedAdders) == observedAdders)
#pragma warning restore 0420
{
Debug.Assert((observedAdders + 1) <= (~COMPLETE_ADDING_ON_MASK), "The number of concurrent adders thread exceeded the maximum limit.");
break;
Expand Down Expand Up @@ -517,9 +516,7 @@ private bool TryAddWithNoTimeValidation(T item, int millisecondsTimeout, Cancell
{
// decrement the adders count
Debug.Assert((_currentAdders & ~COMPLETE_ADDING_ON_MASK) > 0);
#pragma warning disable 0420 // No warning for Interlocked.xxx if compiled with new managed compiler (Roslyn)
Interlocked.Decrement(ref _currentAdders);
#pragma warning restore 0420
}
}
return waitForSemaphoreWasSuccessful;
Expand Down Expand Up @@ -1489,9 +1486,8 @@ public void CompleteAdding()
while (_currentAdders != COMPLETE_ADDING_ON_MASK) spinner.SpinOnce();
return;
}
#pragma warning disable 0420 // No warning for Interlocked.xxx if compiled with new managed compiler (Roslyn)

if (Interlocked.CompareExchange(ref _currentAdders, observedAdders | COMPLETE_ADDING_ON_MASK, observedAdders) == observedAdders)
#pragma warning restore 0420
{
spinner.Reset();
while (_currentAdders != COMPLETE_ADDING_ON_MASK) spinner.SpinOnce();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ public void CopyTo(T[] array, int index)
ToList().CopyTo(array, index);
}

#pragma warning disable 0420 // No warning for Interlocked.xxx if compiled with new managed compiler (Roslyn)
/// <summary>
/// Inserts an object at the top of the <see cref="ConcurrentStack{T}"/>.
/// </summary>
Expand Down Expand Up @@ -662,7 +661,6 @@ private int TryPopCore(int count, out Node poppedHead)
}
}
}
#pragma warning restore 0420

/// <summary>
/// Local helper function to copy the popped elements into a given collection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,6 @@ IEnumerator IEnumerable.GetEnumerator()
return ((InternalPartitionEnumerable)this).GetEnumerator();
}

#pragma warning disable 0420 // No warning for Interlocked.xxx if compiled with new managed compiler (Roslyn)
///////////////////
//
// Used by GrabChunk_Buffered()
Expand Down Expand Up @@ -939,8 +938,11 @@ override protected bool GrabNextChunk(int requestedChunkSize)
{
_localList = new KeyValuePair<long, TSource>[_maxChunkSize];
}

#pragma warning disable 0420 // TODO: https://github.com/dotnet/corefx/issues/35022
// make the actual call to the enumerable that grabs a chunk
return _enumerable.GrabChunk(_localList, requestedChunkSize, ref _currentChunkSize.Value);
#pragma warning restore 0420
}

/// <summary>
Expand Down Expand Up @@ -987,7 +989,6 @@ override public void Dispose()
}
}
#endregion
#pragma warning restore 0420
}
#endregion

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,7 @@ protected override void Dispose(bool disposing)
{
if (disposing)
{
// NOTE : According to https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs0420, the warning is bogus when used with Interlocked API.
#pragma warning disable 420
if (Interlocked.CompareExchange(ref _isDisposed, 1, 0) == 0)
#pragma warning restore 420
{
_catalogs.Dispose();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,7 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
// NOTE : According to https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs0420, the warning is bogus when used with Interlocked API.
#pragma warning disable 420
if (Interlocked.CompareExchange(ref _isDisposed, 1, 0) == 0)
#pragma warning restore 420
{
foreach (ExportProvider provider in _providers)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,7 @@ protected override void Dispose(bool disposing)
{
if (disposing)
{
// NOTE : According to https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs0420, the warning is bogus when used with Interlocked API.
#pragma warning disable 420
if (Interlocked.CompareExchange(ref _isDisposed, 1, 0) == 0)
#pragma warning restore 420
{
INotifyComposablePartCatalogChanged notifyCatalog = _catalog as INotifyComposablePartCatalogChanged;
if (notifyCatalog != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ public virtual IQueryable<ComposablePartDefinition> Parts
{
// Guarantee one time only set _queryableParts
var p = this.AsQueryable();
// NOTE : According to https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs0420, the warning is bogus when used with Interlocked API.
#pragma warning disable 420
Interlocked.CompareExchange(ref _queryableParts, p, null);
#pragma warning restore 420
if (_queryableParts == null)
{
throw new Exception(SR.Diagnostic_InternalExceptionMessage);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,7 @@ public object Value
if (_exportedValue == Export._EmptyValue)
{
object exportedValue = GetExportedValueCore();

// NOTE : According to https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs0420, the warning is bogus when used with Interlocked API.
#pragma warning disable 420
Interlocked.CompareExchange(ref _exportedValue, exportedValue, Export._EmptyValue);
#pragma warning restore 420
}

return _exportedValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ protected override void EnsureRunning()

void IDisposable.Dispose()
{
// NOTE : According to https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs0420, the warning is bogus when used with Interlocked API.
#pragma warning disable 420
if (Interlocked.CompareExchange(ref _isDisposed, 1, 0) == 0)
#pragma warning restore 420
{
ReleaseInstanceIfNecessary(CachedInstance);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,10 @@ internal bool this[int bit]
if (value) newData = oldData | bit;
else newData = oldData & ~bit;

#pragma warning disable 0420
int result = Interlocked.CompareExchange(ref _data, newData, oldData);
#pragma warning restore 0420

if (result == oldData) break;
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2796,8 +2796,6 @@ private void ReadAsyncCallbackCaptureException(TaskCompletionSource<object> sour
}
}

#pragma warning disable 0420 // a reference to a volatile field will not be treated as volatile

public void WriteAsyncCallback(PacketHandle packet, uint sniError)
{
WriteAsyncCallback(IntPtr.Zero, packet, sniError);
Expand Down Expand Up @@ -2885,8 +2883,6 @@ public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError)
}
}

#pragma warning restore 0420

/////////////////////////////////////////
// Network/Packet Writing & Processing //
/////////////////////////////////////////
Expand Down Expand Up @@ -2923,13 +2919,11 @@ internal void ResetSecurePasswordsInformation()
internal Task WaitForAccumulatedWrites()
{
// Checked for stored exceptions
#pragma warning disable 420 // A reference to a volatile field will not be treated as volatile - Disabling since the Interlocked APIs are volatile aware
var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null);
if (delayedException != null)
{
throw delayedException;
}
#pragma warning restore 420

if (_asyncWriteCount == 0)
{
Expand All @@ -2949,13 +2943,11 @@ internal Task WaitForAccumulatedWrites()
}

// Check for stored exceptions
#pragma warning disable 420 // A reference to a volatile field will not be treated as volatile - Disabling since the Interlocked APIs are volatile aware
delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null);
if (delayedException != null)
{
throw delayedException;
}
#pragma warning restore 420

// If there are no outstanding writes, see if we can shortcut and return null
if ((_asyncWriteCount == 0) && ((!task.IsCompleted) || (task.Exception == null)))
Expand Down Expand Up @@ -3212,8 +3204,6 @@ private void CancelWritePacket()
}
}

#pragma warning disable 0420 // a reference to a volatile field will not be treated as volatile

private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock)
{
// Check for a stored exception
Expand Down Expand Up @@ -3351,8 +3341,6 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu

internal abstract uint WritePacket(PacketHandle packet, bool sync);

#pragma warning restore 0420

// Sends an attention signal - executing thread will consume attn.
internal void SendAttention(bool mustTakeWriteLock = false)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@
using System.Threading;
using System.Collections.Generic;

// TODO when we upgrade to C# V6 you can remove this.
// warning CS0420: 'P.x': a reference to a volatile field will not be treated as volatile
// This happens when you pass a _subcribers (a volatile field) to interlocked operations (which are byref).
// This was fixed in C# V6.
#pragma warning disable 0420

namespace System.Diagnostics
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,7 @@ private void EnqueueChunk(T[] chunk)
// write; the CLR 2.0 memory model ensures the write won't move before the write to the
// corresponding element, so a consumer won't see the new index but the corresponding
// element in the array as empty.
#pragma warning disable 0420
Interlocked.Exchange(ref _producerBufferIndex, (bufferIndex + 1) % _buffer.Length);
#pragma warning restore 0420

// (If there is a consumer waiting, we have to ensure to signal the event. Unfortunately,
// this requires that we issue a memory barrier: We need to guarantee that the write to
Expand Down Expand Up @@ -338,9 +336,7 @@ private void WaitUntilNonFull()
// very quickly, suddenly seeing an empty queue. This would lead to deadlock
// if we aren't careful. Therefore we check the empty/full state AGAIN after
// setting our flag to see if a real wait is warranted.
#pragma warning disable 0420
Interlocked.Exchange(ref _producerIsWaiting, 1);
#pragma warning restore 0420

// (We have to prevent the reads that go into determining whether the buffer
// is full from moving before the write to the producer-wait flag. Hence the CAS.)
Expand Down Expand Up @@ -557,9 +553,7 @@ private bool TryDequeueChunk(ref T[] chunk, ref bool isDone)
// very quickly, suddenly seeing a full queue. This would lead to deadlock
// if we aren't careful. Therefore we check the empty/full state AGAIN after
// setting our flag to see if a real wait is warranted.
#pragma warning disable 0420
Interlocked.Exchange(ref _consumerIsWaiting, 1);
#pragma warning restore 0420

// (We have to prevent the reads that go into determining whether the buffer
// is full from moving before the write to the producer-wait flag. Hence the CAS.)
Expand Down Expand Up @@ -619,9 +613,7 @@ private T[] InternalDequeueChunk()
// write; the CLR 2.0 memory model ensures the write won't move before the write to the
// corresponding element, so a consumer won't see the new index but the corresponding
// element in the array as empty.
#pragma warning disable 0420
Interlocked.Exchange(ref _consumerBufferIndex, (consumerBufferIndex + 1) % _buffer.Length);
#pragma warning restore 0420

// (Unfortunately, this whole sequence requires a memory barrier: We need to guarantee
// that the write to _consumerBufferIndex doesn't pass the read of the wait-flags; the CLR memory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ internal bool this[int bit]
newData = oldData & ~bit;
}

#pragma warning disable 0420
int result = Interlocked.CompareExchange(ref _data, newData, oldData);
#pragma warning restore 0420

if (result == oldData)
{
break;
Expand Down Expand Up @@ -69,10 +66,7 @@ internal bool ChangeValue(int bit, bool value)
return false;
}

#pragma warning disable 0420
int result = Interlocked.CompareExchange(ref _data, newData, oldData);
#pragma warning restore 0420

if (result == oldData)
{
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ internal bool ProcessCompletedOperationInCallback
}


#pragma warning disable 420 // "a reference to a volatile field will not be treated as volatile"

public WaitHandle AsyncWaitHandle
{
get
Expand All @@ -97,9 +95,6 @@ public WaitHandle AsyncWaitHandle
}
}

#pragma warning restore 420 // "a reference to a volatile field will not be treated as volatile"


public bool CompletedSynchronously
{
get { return false; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ internal class TaskToAsyncInfoAdapter<TCompletedHandler, TProgressHandler, TResu
where TCompletedHandler : class
where TProgressHandler : class
{
// This class uses interlocked operations on volatile fields, and this pragma suppresses the compiler's complaint
// that passing a volatile by ref to a function removes the volatility. That's not necessary for interlocked.
#pragma warning disable 0420

#region Private Types, Statics and Constants

// ! THIS DIAGRAM ILLUSTRATES THE CONSTANTS BELOW. UPDATE THIS IF UPDATING THE CONSTANTS BELOW!:
Expand Down Expand Up @@ -1023,9 +1019,6 @@ public virtual AsyncStatus Status
}
}
#endregion Implementation of IAsyncInfo

#pragma warning restore 0420

} // class TaskToAsyncInfoAdapter<TCompletedHandler, TProgressHandler, TResult, TProgressInfo>
} // namespace

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
#pragma warning disable 0420


// =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -769,9 +769,7 @@ private void OfferAsyncIfNecessary_Slow(bool isReplacementReplica, bool outgoing
#endif

// Start the task handling scheduling exceptions
#pragma warning disable 0420
Exception exception = Common.StartTaskSafe(_taskForOutputProcessing, _dataflowBlockOptions.TaskScheduler);
#pragma warning restore 0420
if (exception != null)
{
// First, log the exception while the processing state is dirty which is preventing the block from completing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
using System.Runtime.CompilerServices;
using System.Security;

#pragma warning disable 0420 // turn off warning for passing volatiles to interlocked operations
namespace System.Threading.Tasks.Dataflow.Internal
{
// SpscTargetCore provides a fast target core for use in blocks that will only have single-producer-single-consumer
Expand Down Expand Up @@ -313,7 +312,9 @@ private void StoreException(Exception exception)
// the exception because this method could be accessed concurrently
// by the producer and consumer, a producer calling Fault and the
// processing task processing the user delegate which might throw.
#pragma warning disable 0420
lock (LazyInitializer.EnsureInitialized(ref _exceptions, () => new List<Exception>()))
#pragma warning restore 0420
{
_exceptions.Add(exception);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,6 @@ internal void Complete(Exception exception, bool dropPendingMessages, bool store
{
Debug.Assert(_numberOfOutstandingOperations > 0 || !storeExceptionEvenIfAlreadyCompleting,
"Calls with storeExceptionEvenIfAlreadyCompleting==true may only be coming from processing task.");

#pragma warning disable 0420
Common.AddException(ref _exceptions, exception, unwrapInnerExceptions);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

// Prevents compiler warnings/errors regarding the use of ref params in Interlocked methods

#pragma warning disable 0420
namespace System.Threading.Tasks
{
/// <summary>
Expand Down Expand Up @@ -624,5 +623,3 @@ public struct ParallelLoopResult
public long? LowestBreakIteration { get { return _lowestBreakIteration; } }
}
}

#pragma warning restore 0420
Loading

0 comments on commit 860ccad

Please sign in to comment.