Skip to content

Commit

Permalink
Respond to PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@agocke
agocke committed Jul 1, 2020
1 parent dfb4de6 commit 8844778
Show file tree
Hide file tree
Showing 5 changed files with 272 additions and 41 deletions.
6 changes: 2 additions & 4 deletions src/Compilers/CSharp/Portable/Lowering/Instrumentation/DynamicAnalysisInjector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,10 +590,8 @@ private static Text.TextSpan SkipAttributes(SyntaxNode syntax)
return SkipAttributes(syntax, propertySyntax.AttributeLists, propertySyntax.Modifiers, default(SyntaxToken), propertySyntax.Type);

case SyntaxKind.DataPropertyDeclaration:
{
var dataProp = (DataPropertyDeclarationSyntax)syntax;
return SkipAttributes(syntax, dataProp.AttributeLists, dataProp.Modifiers, dataProp.DataKeyword, dataProp.Type);
}
Debug.Assert(false, "data properties cannot contain bodies");
break;

case SyntaxKind.GetAccessorDeclaration:
case SyntaxKind.SetAccessorDeclaration:
Expand Down
5 changes: 4 additions & 1 deletion src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ public static bool IsInTypeOnlyContext(ExpressionSyntax node)
case PropertyDeclaration:
return ((PropertyDeclarationSyntax)parent).Type == node;

case DataPropertyDeclaration:
return ((DataPropertyDeclarationSyntax)parent).Type == node;

case DelegateDeclaration:
return ((DelegateDeclarationSyntax)parent).ReturnType == node;

Expand Down Expand Up @@ -254,7 +257,7 @@ public static bool IsInNamespaceOrTypeContext(ExpressionSyntax? node)
}

/// <summary>
/// Is the node the name of a named argument of an invocation, object creation expression,
/// Is the node the name of a named argument of an invocation, object creation expression,
/// constructor initializer, or element access, but not an attribute.
/// </summary>
public static bool IsNamedArgumentName(SyntaxNode node)
Expand Down
14 changes: 10 additions & 4 deletions src/Compilers/CSharp/Test/Emit/Emit/DynamicAnalysis/DynamicAnalysisResourceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ public static void Main() // Method 0
{
Fred();
}
[Obsolete()]
static void Fred() // Method 1
{
Expand Down Expand Up @@ -341,7 +341,7 @@ ref int BamBam(ref int x) // Method 7
var reader = DynamicAnalysisDataReader.TryCreateFromPE(peReader, "<DynamicAnalysisData>");

VerifyDocuments(reader, reader.Documents,
@"'C:\myproject\doc1.cs' A3-08-94-55-7C-64-8D-C7-61-7A-11-0B-4B-68-2C-3B-51-C3-C4-58 (SHA1)");
@"'C:\myproject\doc1.cs' A4-02-50-34-9F-CD-91-F7-CB-94-31-74-4C-E6-71-07-8E-8A-F9-DD (SHA1)");

Assert.Equal(15, reader.Methods.Length);

Expand Down Expand Up @@ -404,7 +404,7 @@ public static void Main() // Method 0
s.GPA = 2.3;
Operate(s);
}
static string Operate(Person p) // Method 1
{
switch (p)
Expand Down Expand Up @@ -611,10 +611,13 @@ static C() // Method 4
int Prop1 { get; } = 15;
static int Prop2 { get; } = 255;
[Obsolete]
data int Prop3 = 16;
}
";

var c = CreateCompilation(Parse(source + InstrumentationHelperSource, @"C:\myproject\doc1.cs"));
var c = CreateCompilation(Parse(source + IsExternalInitTypeDefinition + InstrumentationHelperSource,
@"C:\myproject\doc1.cs", options: TestOptions.RegularPreview));
var peImage = c.EmitToArray(EmitOptions.Default.WithInstrumentationKinds(ImmutableArray.Create(InstrumentationKind.TestCoverage)));

var peReader = new PEReader(peImage);
Expand All @@ -640,6 +643,7 @@ static C() // Method 4
new SpanResult(27, 13, 27, 19, "Init()"),
new SpanResult(28, 13, 28, 24, "Init() + 12"),
new SpanResult(44, 25, 44, 27, "15"),
new SpanResult(47, 21, 47, 23, "16"),
new SpanResult(19, 8, 19, 16, "_z = 12"));

VerifySpans(reader, reader.Methods[4], sourceLines,
Expand All @@ -654,13 +658,15 @@ static C() // Method 4
new SpanResult(27, 13, 27, 19, "Init()"),
new SpanResult(28, 13, 28, 24, "Init() + 12"),
new SpanResult(44, 25, 44, 27, "15"),
new SpanResult(47, 21, 47, 23, "16"),
new SpanResult(36, 8, 36, 15, "_z = x"));

VerifySpans(reader, reader.Methods[6], sourceLines,
new SpanResult(39, 4, 42, 5, "C(int a, int b)"),
new SpanResult(27, 13, 27, 19, "Init()"),
new SpanResult(28, 13, 28, 24, "Init() + 12"),
new SpanResult(44, 25, 44, 27, "15"),
new SpanResult(47, 21, 47, 23, "16"),
new SpanResult(41, 8, 41, 19, "_z = a + b"));
}

Expand Down
119 changes: 89 additions & 30 deletions src/Compilers/CSharp/Test/Emit/Emit/DynamicAnalysis/DynamicInstrumentationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,65 @@ namespace Microsoft.CodeAnalysis.CSharp.DynamicAnalysis.UnitTests
{
public class DynamicInstrumentationTests : CSharpTestBase
{
[Fact]
public void DataPropertyInitializer()
{
string source = @"
using System;
using System.Diagnostics.CodeAnalysis;
class C
{
data int P1;
data int P2 = 25;
public static void Main(string[] args)
{
new C();
Microsoft.CodeAnalysis.Runtime.Instrumentation.FlushPayload();
}
}
";
var verifier = CompileAndVerify(source + InstrumentationHelperSource + IsExternalInitTypeDefinition,
options: TestOptions.ReleaseExe, parseOptions: TestOptions.RegularPreview);

// data accessors are implicit, therefore not instrumented
AssertNotInstrumented(verifier, "C.P1.get");
AssertNotInstrumented(verifier, "C.P1.init");

verifier.VerifyIL("C..ctor()", @"
{
// Code size 71 (0x47)
.maxstack 5
.locals init (bool[] V_0)
IL_0000: ldsfld ""bool[][] <PrivateImplementationDetails>.PayloadRoot0""
IL_0005: ldtoken ""C..ctor()""
IL_000a: ldelem.ref
IL_000b: stloc.0
IL_000c: ldloc.0
IL_000d: brtrue.s IL_0034
IL_000f: ldsfld ""System.Guid <PrivateImplementationDetails>.MVID""
IL_0014: ldtoken ""C..ctor()""
IL_0019: ldtoken Source Document 0
IL_001e: ldsfld ""bool[][] <PrivateImplementationDetails>.PayloadRoot0""
IL_0023: ldtoken ""C..ctor()""
IL_0028: ldelema ""bool[]""
IL_002d: ldc.i4.1
IL_002e: call ""bool[] Microsoft.CodeAnalysis.Runtime.Instrumentation.CreatePayload(System.Guid, int, int, ref bool[], int)""
IL_0033: stloc.0
IL_0034: ldloc.0
IL_0035: ldc.i4.0
IL_0036: ldc.i4.1
IL_0037: stelem.i1
IL_0038: ldarg.0
IL_0039: ldc.i4.s 25
IL_003b: stfld ""int C.<P2>k__BackingField""
IL_0040: ldarg.0
IL_0041: call ""object..ctor()""
IL_0046: ret
}");
}

[Fact]
public void HelpersInstrumentation()
{
Expand Down Expand Up @@ -766,7 +825,7 @@ public static void Main(string[] args) // Metho
TestMain();
Microsoft.CodeAnalysis.Runtime.Instrumentation.FlushPayload();
}
static void TestMain() // Method 2
{
int x = Count;
Expand Down Expand Up @@ -873,11 +932,11 @@ public static void Main(string[] args) // Metho
TestMain();
Microsoft.CodeAnalysis.Runtime.Instrumentation.FlushPayload();
}
static void TestMain() // Method 2
{
new D().M1();
}
}
}
public class D
Expand Down Expand Up @@ -971,7 +1030,7 @@ public static void Main(string[] args) // Metho
TestMain();
Microsoft.CodeAnalysis.Runtime.Instrumentation.FlushPayload();
}
#line 20 ""File2.cs""
static void TestMain() // Method 2
{
Expand Down Expand Up @@ -1442,7 +1501,7 @@ static void TestMain() // Method 2
s.GPA = s.Name switch { _ => 2.3 }; // switch expression is not instrumented
Operate(s);
}
static string Operate(Person p) // Method 3
{
switch (p)
Expand Down Expand Up @@ -1866,7 +1925,7 @@ File 1
public void IteratorCoverage()
{
string source = @"
using System;
using System;
public class Program
{
Expand All @@ -1879,11 +1938,11 @@ public static void Main(string[] args) // Metho
static void TestMain() // Method 2
{
foreach (var i in Goo())
{
{
Console.WriteLine(i);
}
}
foreach (var i in Goo())
{
{
Console.WriteLine(i);
}
}
Expand Down Expand Up @@ -2280,14 +2339,14 @@ public void MissingMethodNeededForAnalysis()
string source = @"
namespace System
{
public class Object { }
public struct Int32 { }
public struct Boolean { }
public class String { }
public class Exception { }
public class ValueType { }
public class Enum { }
public struct Void { }
public class Object { }
public struct Int32 { }
public struct Boolean { }
public class String { }
public class Exception { }
public class ValueType { }
public class Enum { }
public struct Void { }
public class Guid { }
}
Expand Down Expand Up @@ -2397,7 +2456,7 @@ class C
{
[ExcludeFromCodeCoverage]
static void M1() { L1(); void L1() { new Action(() => { Console.WriteLine(1); }).Invoke(); } }
static void M2() { L2(); void L2() { new Action(() => { Console.WriteLine(2); }).Invoke(); } }
}
";
Expand Down Expand Up @@ -2561,16 +2620,16 @@ public void ExcludeFromCodeCoverageAttribute_LocalFunctionsAndLambdas_InAccessor
class C
{
[ExcludeFromCodeCoverage]
int P1
{
get { L1(); void L1() { Console.WriteLine(1); } return 1; }
set { L2(); void L2() { Console.WriteLine(2); } }
int P1
{
get { L1(); void L1() { Console.WriteLine(1); } return 1; }
set { L2(); void L2() { Console.WriteLine(2); } }
}
int P2
{
get { L3(); void L3() { Console.WriteLine(3); } return 3; }
set { L4(); void L4() { Console.WriteLine(4); } }
{
get { L3(); void L3() { Console.WriteLine(3); } return 3; }
set { L4(); void L4() { Console.WriteLine(4); } }
}
}
";
Expand Down Expand Up @@ -2712,10 +2771,10 @@ class C
{
[ExcludeFromCodeCoverage]
int P1 { get => 1; set {} }
[ExcludeFromCodeCoverage]
event Action E1 { add { } remove { } }
int P2 { get => 1; set {} }
event Action E2 { add { } remove { } }
}
Expand Down Expand Up @@ -2842,7 +2901,7 @@ public void TestPartialMethodsWithImplementation()
public partial class Class1<T>
{
partial void Method1<U>(int x);
public void Method2(int x)
public void Method2(int x)
{
Console.WriteLine($""Method2: x = {x}"");
Method1<T>(x);
Expand Down Expand Up @@ -2944,7 +3003,7 @@ public void TestPartialMethodsWithoutImplementation()
public partial class Class1<T>
{
partial void Method1<U>(int x);
public void Method2(int x)
public void Method2(int x)
{
Console.WriteLine($""Method2: x = {x}"");
Method1<T>(x);
Expand Down Expand Up @@ -3391,7 +3450,7 @@ private static void AssertInstrumented(CompilationVerifier verifier, string qual
{
string il = verifier.VisualizeIL(qualifiedMethodName);

// Tests using this helper are constructed such that instrumented methods contain a call to CreatePayload,
// Tests using this helper are constructed such that instrumented methods contain a call to CreatePayload,
// lambdas a reference to payload bool array.
bool instrumented = il.Contains("CreatePayload") || il.Contains("bool[]");

Expand Down
Loading

0 comments on commit 8844778

Please sign in to comment.