Skip to content

Commit

Permalink
Address more issue numbers in the code (#1888)
Browse files Browse the repository at this point in the history
* Address more issue numbers in the code

Replaced a bunch of unadorned issue numbers with links, so we know which repo the issue is for.  In some cases the repo was named, but for consistency I still replaced it with a link.

In some cases, in particular where the issue had been closed saying effectively "we're not going to do anything here", I removed TODOs and the associated issue numbers.

In a few cases, the TODO and issue number was to say "we should do X when Y is fixed", so I did X since Y was fixed.

* Apply suggestions from code review

Co-Authored-By: Jan Kotas <jkotas@microsoft.com>

* Address PR feedback

* Fix OrderedEnumerable by using MemoryExtensions.Sort

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
  • Loading branch information
stephentoub and jkotas committed Jan 18, 2020
1 parent 522e9d9 commit ccf6aed
Show file tree
Hide file tree
Showing 349 changed files with 653 additions and 750 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace System.Reflection.Emit
// This file implements the minimum subset of ISymWrapper.dll required to restore
// that functionality. Namely, the SymWriter and SymDocumentWriter objects.
//
// Ideally we wouldn't need ISymWrapper.dll on desktop either - it's an ugly piece
// Ideally we wouldn't need ISymWrapper.dll on .NET Framework either - it's an ugly piece
// of legacy. We could just use this (or COM-interop code) everywhere, but we might
// have to worry about compatibility.
//
Expand All @@ -39,7 +39,7 @@ private SymWrapperCore()
}

//------------------------------------------------------------------------------
// Implements Telesto's version of SymDocumentWriter (in the desktop world,
// Implements Telesto's version of SymDocumentWriter (in the .NET Framework world,
// this type is exposed from ISymWrapper.dll.)
//
// The only thing user code can do with this wrapper is to receive it from
Expand Down Expand Up @@ -77,7 +77,7 @@ internal PunkSafeHandle GetUnmanaged()
//------------------------------------------------------------------------------
void ISymbolDocumentWriter.SetSource(byte[] source)
{
throw new NotSupportedException(); // Intentionally not supported to match desktop CLR
throw new NotSupportedException(); // Intentionally not supported to match .NET Framework CLR
}

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -136,7 +136,7 @@ private struct ISymUnmanagedDocumentWriter


//------------------------------------------------------------------------------
// Implements Telesto's version of SymWriter (in the desktop world,
// Implements Telesto's version of SymWriter (in the .NET Framework world,
// this type is expored from ISymWrapper.dll.)
//------------------------------------------------------------------------------
internal unsafe class SymWriter : ISymbolWriter
Expand All @@ -153,7 +153,7 @@ internal static ISymbolWriter CreateSymWriter()

//------------------------------------------------------------------------------
// Basic ctor. You'd think this ctor would take the unmanaged symwriter object as an argument
// but to fit in with existing desktop code, the unmanaged writer is passed in
// but to fit in with existing .NET Framework code, the unmanaged writer is passed in
// through a subsequent call to InternalSetUnderlyingWriter
//------------------------------------------------------------------------------
private SymWriter()
Expand Down Expand Up @@ -250,7 +250,7 @@ void ISymbolWriter.DefineSequencePoints(ISymbolDocumentWriter document,
}

// Sure, claim to accept any type that implements ISymbolDocumentWriter but the only one that actually
// works is the one returned by DefineDocument. The desktop ISymWrapper commits the same signature fraud.
// works is the one returned by DefineDocument. The .NET Framework ISymWrapper commits the same signature fraud.
// Ideally we'd just return a sealed opaque cookie type, which had an internal accessor to
// get the writer out.
// Regardless, this cast is important for security - we cannot allow our caller to provide
Expand Down Expand Up @@ -497,7 +497,7 @@ private struct ISymUnmanagedWriter
//
// ! Because the Release occurs in the finalizer thread, this safehandle really takes
// ! an ostrich approach to apartment issues. We only tolerate this here because we're emulating
// ! the desktop CLR's use of ISymWrapper which also pays lip service to COM apartment rules.
// ! the .NET Framework CLR's use of ISymWrapper which also pays lip service to COM apartment rules.
// !
// ! However, think twice about pulling this safehandle out for other uses.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ private static SizeAndAlignment ComputeFieldSizeAndAlignment(TypeDesc fieldType,

private static int ComputePackingSize(MetadataType type, ClassLayoutMetadata layoutMetadata)
{
// If a type contains pointers then the metadata specified packing size is ignored (On desktop this is disqualification from ManagedSequential)
// If a type contains pointers then the metadata specified packing size is ignored (On .NET Framework this is disqualification from ManagedSequential)
if (layoutMetadata.PackingSize == 0 || type.ContainsGCPointers)
return type.Context.Target.DefaultPackingSize;
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
namespace Internal.TypeSystem.Ecma
{
/// <summary>
/// Provides PdbSymbolReader via unmanaged SymBinder from full .NET Framework
/// Provides PdbSymbolReader via unmanaged SymBinder from .NET Framework
/// </summary>
public sealed class UnmanagedPdbSymbolReader : PdbSymbolReader
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ internal static MarshallerKind GetMarshallerKind(
//
// Determine MarshalerKind
//
// This mostly resembles desktop CLR and .NET Native code as we need to match their behavior
// This mostly resembles .NET Framework CLR and .NET Native code as we need to match their behavior
//
if (type.IsPrimitive)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ public int GetNextOffset()

Debug.Assert(!IsVarArg || !HasParamType);

// DESKTOP BEHAVIOR - This block is disabled for x86 as the param arg is the last argument on desktop x86.
// DESKTOP BEHAVIOR - This block is disabled for x86 as the param arg is the last argument on .NET Framework x86.
if (HasParamType)
{
numRegistersUsed++;
Expand Down Expand Up @@ -1325,7 +1325,7 @@ private void ForceSigWalk()
nSizeOfArgStack += _transitionBlock.PointerSize;
}

// DESKTOP BEHAVIOR - This block is disabled for x86 as the param arg is the last argument on desktop x86.
// DESKTOP BEHAVIOR - This block is disabled for x86 as the param arg is the last argument on .NET Framework x86.
if (HasParamType)
{
numRegistersUsed++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ public bool InputArchitectureSupported()

// TODO: Fix R2RDump issue where an R2R image cannot be dissassembled with the x86 CoreDisTools
// For the short term, we want to error out with a decent message explaining the unexpected error
// Issue #19564: https://github.com/dotnet/coreclr/issues/19564
// Issue https://github.com/dotnet/coreclr/issues/19564
public bool DisassemblerArchitectureSupported()
{
System.Runtime.InteropServices.Architecture val = System.Runtime.InteropServices.RuntimeInformation.ProcessArchitecture;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ unsafe static int Main(string[] args)
{
HostPolicyMock.Initialize(Environment.CurrentDirectory, null);

// Load our fake mscoree to prevent desktop from loading.
// Load our fake mscoree to prevent .NET Framework from loading.
NativeLibrary.Load(Path.Combine(Environment.CurrentDirectory, "mscoree.dll"));

Console.WriteLine("Verify that we can load an IJW assembly from native code.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

// The test came from #20085. It tries to access field from a promoted struct with an offset that
// The test came from https://github.com/dotnet/corefx/issues/20085.
// It tries to access field from a promoted struct with an offset that
// is not valid for the promoted struct type.

using System.Runtime.CompilerServices;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

// The test came from #20085. It tests that we do access overlapping fields with the correct types.
// The test came from https://github.com/dotnet/corefx/issues/20085.
// It tests that we do access overlapping fields with the correct types.
// Espessialy if the stuct was casted by 'Unsafe.As` from a promoted type
// and the promoted type had another field on the same offset but with a different type/size.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

// The test came from #20085. It tests that we do access overlapping fields with the correct types.
// The test came from https://github.com/dotnet/corefx/issues/20085.
// It tests that we do access overlapping fields with the correct types.
// Espessialy if the stuct was casted by 'Unsafe.As` from a promoted type
// and the promoted type had another field on the same offset but with a different type/size.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public struct S20
public long z;
public long w;
}
/* These tests are not working on non Windows CoreCLR. Enable this when GH Issue #2076 is resolved.
/* These tests are not working on non Windows CoreCLR. Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved.
[StructLayout(LayoutKind.Sequential)]
public struct S28
{
Expand All @@ -185,7 +185,7 @@ public struct S29
public int x;
public object y;
}
Enable this when GH Issue #2076 is resolved. */
Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved. */
public struct S30
{
public long x;
Expand Down Expand Up @@ -213,10 +213,10 @@ public struct S30
public delegate void MyCallback19(S19 s);
public delegate void MyCallback20(S20 s);

/* These tests are not working on non Windows CoreCLR. Enable this when GH Issue #2076 is resolved.
/* These tests are not working on non Windows CoreCLR. Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved.
public delegate void MyCallback28(S28 s);
public delegate void MyCallback29(S29 s);
Enable this when GH Issue #2076 is resolved. */
Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved. */
public delegate void MyCallback30(S30 s1, S30 s2, S30 s3);

[DllImport("jitstructtests_lib")]
Expand Down Expand Up @@ -278,14 +278,14 @@ Enable this when GH Issue #2076 is resolved. */

[DllImport("jitstructtests_lib")]
public static extern void InvokeCallback20(MyCallback20 callback, S20 s);
/* These tests are not working on non Windows CoreCLR. Enable this when GH Issue #2076 is resolved.
/* These tests are not working on non Windows CoreCLR. Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved.
[DllImport("jitstructtests_lib")]
public static extern void InvokeCallback28(MyCallback28 callback, S28 s);
[DllImport("jitstructtests_lib")]
public static extern void InvokeCallback29(MyCallback29 callback, S29 s);
Enable this when GH Issue #2076 is resolved. */
Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved. */
[DllImport("jitstructtests_lib")]
public static extern void InvokeCallback30(MyCallback30 callback, S30 s1, S30 s2, S30 s3);

Expand Down Expand Up @@ -348,14 +348,14 @@ Enable this when GH Issue #2076 is resolved. */

[DllImport("jitstructtests_lib")]
public static extern S20 InvokeCallback20R(MyCallback20 callback, S20 s);
/* These tests are not working on non Windows CoreCLR. Enable this when GH Issue #2076 is resolved.
/* These tests are not working on non Windows CoreCLR. Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved.
[DllImport("jitstructtests_lib")]
public static extern S28 InvokeCallback28R(MyCallback28 callback, S28 s);
[DllImport("jitstructtests_lib")]
public static extern S29 InvokeCallback29R(MyCallback29 callback, S29 s);
Enable this when GH Issue #2076 is resolved. */
Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved. */
static public int Main1()
{
Program3 p = new Program3();
Expand Down Expand Up @@ -642,7 +642,7 @@ static public int Main1()
}
}, s20);

/* These tests are not working on non Windows CoreCLR. Enable this when GH Issue #2076 is resolved.
/* These tests are not working on non Windows CoreCLR. Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved.
TestClass testClass = new TestClass();
S28 s28;
s28.x = null;
Expand Down Expand Up @@ -689,7 +689,7 @@ static public int Main1()
throw new System.Exception();
}
}, s29);
Enable this when GH Issue #2076 is resolved. */
Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved. */
S30 s30;
s30.x = 1;
s30.y = 2;
Expand Down Expand Up @@ -990,7 +990,7 @@ Enable this when GH Issue #2076 is resolved. */
{
throw new System.Exception();
}
/* These tests are not working on non Windows CoreCLR. Enable this when GH Issue #2076 is resolved.
/* These tests are not working on non Windows CoreCLR. Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved.
s28.x = null;
S28 s28r = InvokeCallback28R((par) => {
Console.WriteLine("S28: {0}, {1}", par.x == null ? "Null" : "Not null", par.y);
Expand Down Expand Up @@ -1049,7 +1049,7 @@ Enable this when GH Issue #2076 is resolved. */
{
throw new System.Exception();
}
Enable this when GH Issue #2076 is resolved. */
Enable this when https://github.com/dotnet/coreclr/issues/2076 is resolved. */
}
catch (Exception e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

[assembly: OptimizeForBenchmarks]

// Test code taken directly from GitHub issue #9692 (https://github.com/dotnet/coreclr/issues/9692)
// Test code taken directly from https://github.com/dotnet/coreclr/issues/9692
// Laying the loop's early return path in-line can cost 30% on this micro-benchmark.

namespace Layout
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ internal class Controls
public bool DoListBenchmarkTagSets; // List out the benchmark tag sets from the .XML file
public bool DoListBenchmarkExecutables; // List out the benchmark exectubles from the .XML file
public int NumberOfRunsPerBenchmark; // Number of runs/iterations each benchmark should be run
public string ComplusVersion; // COMPlus_VERSION for desktop CLR hosted runs (optional).
public string ComplusVersion; // COMPlus_VERSION for .NET Framework CLR hosted runs (optional).
public string BenchmarksRootDirectory; // Root directory for benchmark tree specified in .XML file.
public string BenchmarkXmlFileName; // Benchmark .XML filename (default coreclr_benchmarks.xml)
public string BenchmarkCsvFileName; // Benchmark output .CSV filename (default coreclr_benchmarks.csv)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// See the LICENSE file in the project root for more information.

// This test illustrates a limitation in the JIT in that it will not promote
// a struct that has a single double register. See GitHub issue #1161.
// a struct that has a single double register. See https://github.com/dotnet/coreclr/issues/1161.

using System;
using System.Runtime.CompilerServices;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

// This test captures the redundant struct zeroing from GitHub issue #11816.
// This test captures the redundant struct zeroing from https://github.com/dotnet/coreclr/issues/11816.
// Since the issue was filed, the 'TestStructManuallyInlined' case has apparently
// gotten worse, as there is a MEMSET of the large struct to 0.

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

using System;

// Test case for issue 18259
// Test case for https://github.com/dotnet/coreclr/issues/18259
//
// We were a missing check for ZeroOffsetFldSeq values on LclVar reads
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

using System;

// Test case for issue 21231
// Test case for https://github.com/dotnet/coreclr/issues/21231
//
//
// Debug: Outputs 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

using System;

// Test case for issue 21231
// Test case for https://github.com/dotnet/coreclr/issues/21231
//
//
// Debug: Outputs 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

using System;

// Repro for issue 22820. On x86 we need to report enclosed handler
// Repro for https://github.com/dotnet/coreclr/issues/22820.
// On x86 we need to report enclosed handler
// live-in locals as live into any enclosing filter.
//
// Run with optimized codegen and COMPlus_GCStress=0x4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

// Issue 2580 fix description
// Issue https://github.com/dotnet/coreclr/issues/2580 fix description
//
// The changes fix a bug generating 16-bit signed comparison code to
// compare unsigned 16-bit values.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

// Since Issue 7508 was a performance issue, there's not really a correctness
// test for this.
// Since https://github.com/dotnet/coreclr/issues/7508 was a performance issue,
// there's not really a correctness test for this.
// However, this is a very simple test that can be used to compare the code generated
// for a non-accelerated vector of 3 floats, a "raw" Vector3 and a Vector3
// wrapped in a struct.
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/tests/src/JIT/SIMD/Vector3GetHash.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
// This was copied from corefx\src\System.Numerics.Vectors\tests\Vector3Tests.cs.
// It exposed a bug in morph, in which a SIMD field was being morphed in
// a MACK_Ind context, even though it was under a GT_IND(G_ADDR()).
// This was corefx issue #15713, and was "fixed" with coreclr PR # 9496,
// and a new issue, #9518, has been filed to track the underlying unexpected
// This was https://github.com/dotnet/corefx/issues/15713, and was "fixed" with
// https://github.com/dotnet/coreclr/issues/9496 and a new issue,
// https://github.com/dotnet/coreclr/issues/9518, has been filed to track the underlying unexpected
// MACK_Ind context, which causes us to mark the Vector3 local as do-not-enregister.

using System;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tests/src/JIT/opt/Structs/structpop.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// Optimization of pop with struct types
// Codegen for TestByRef and TestByPtr should be similar
//
// See CoreClr#18710
// See https://github.com/dotnet/coreclr/issues/18710

using System;
using System.Runtime.CompilerServices;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tests/src/JIT/opt/Structs/structpop2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// Optimization of pop with struct types
// Codegen for TestByPtr should be minimal
//
// See CoreClr#18710
// See https://github.com/dotnet/coreclr/issues/18710

using System;
using System.Runtime.CompilerServices;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
using System;
using System.Runtime.CompilerServices;

// Repro case for CoreCLR 17398
// Repro case for https://github.com/dotnet/coreclr/pull/17398

class X
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

// Test Description:
// Just basic heavy reading and writing from ThreadStatic members in normal threads and threadpools threads as well.
// Ported from desktop test: BaseServices\Regression\V1\Threads\ThreadStatic\threadstatic1.cs
// Ported from .NET Framework test: BaseServices\Regression\V1\Threads\ThreadStatic\threadstatic1.cs

public class Sensor
{
Expand Down
Loading

0 comments on commit ccf6aed

Please sign in to comment.