Report diagnostic on correct node by Youssef1313 · Pull Request #53538 · dotnet/roslyn

Youssef1313 · 2021-05-20T10:29:06Z

davidwengier · 2021-05-20T12:05:43Z

src/Compilers/CSharp/Test/Emit/CodeGen/IndexAndRangeTests.cs

                // (12,27): error CS0029: Cannot implicitly convert type 'System.Index' to 'int'
                //         Console.WriteLine(s[new Index(1, false), ^1]);
-                Diagnostic(ErrorCode.ERR_NoImplicitConv, "s[new Index(1, false), ^1]").WithArguments("System.Index", "int").WithLocation(12, 27));
+                Diagnostic(ErrorCode.ERR_NoImplicitConv, "^1").WithArguments("System.Index", "int").WithLocation(12, 27));


Do the locations need to be updated too?

Yep I forgot those. Will update them shortly.

davidwengier

Thanks @Youssef1313 😀

IDE test change LGTM

AlekseyTs · 2021-05-20T13:49:16Z

src/Compilers/CSharp/Test/Semantic/Semantics/IndexAndRangeTests.cs

                // (11,17): error CS0656: Missing compiler required member 'System.Runtime.CompilerServices.RuntimeHelpers.GetSubArray'
                //         var x = arr[0..2];
-                Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "arr[0..2]").WithArguments("System.Runtime.CompilerServices.RuntimeHelpers", "GetSubArray").WithLocation(11, 17));
+                Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "0..2").WithArguments("System.Runtime.CompilerServices.RuntimeHelpers", "GetSubArray").WithLocation(11, 21));


Diagnostic

It looks like we are lacking test coverage for the following scenarios affected by the change:

pointer indexing

array creation

variable declaration with dimensions on the type #Closed

Will add a test for pointer indexing.

Array creation is covered in `CS0266ERR_NoImplicitConvCast13:

roslyn/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs

Lines 9235 to 9244 in 67c6bed

VerifyDiagnostics(

// (7,30): error CS0266: Cannot implicitly convert type 'double' to 'int'. An explicit conversion exists (are you missing a cast?)

// int[] arr4 = new int[x];// Invalid

Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "x").WithArguments("double", "int").WithLocation(7, 30),

// (10,30): error CS0266: Cannot implicitly convert type 'float' to 'int'. An explicit conversion exists (are you missing a cast?)

// int[] arr5 = new int[y];// Invalid

Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "y").WithArguments("float", "int").WithLocation(10, 30),

// (13,30): error CS0266: Cannot implicitly convert type 'decimal' to 'int'. An explicit conversion exists (are you missing a cast?)

// int[] arr6 = new int[z];// Invalid

Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "z").WithArguments("decimal", "int").WithLocation(13, 30));

If you mean int[o] arr;, then this will produce error CS0270: Array size cannot be specified in a variable declaration (try initializing with a 'new' expression).

If you mean int[o] arr;, then this will produce error CS0270: Array size cannot be specified in a variable declaration (try initializing with a 'new' expression).

Do we have an existing test demonstrating this with type mismatch in the illegal sizes?

@AlekseyTs If you mean something like:

public void M(object o) { int[o] x; }

Then I think there is no test for it. I'll add it once you confirm this is the case you're requesting.

If you mean something like ...

Yes, something like this. We want a test that hits your changes in BindArrayDimension for a variable declaration

AlekseyTs · 2021-05-20T13:50:28Z

@Youssef1313 It looks like there are legitimate test failures.

AlekseyTs · 2021-05-20T13:51:55Z

Done with review pass (commit 4)

In reply to: 845142404

AlekseyTs · 2021-05-20T14:44:27Z

Done with review pass (commit 6)

In reply to: 845185257

AlekseyTs

LGTM (commit 8)

AlekseyTs · 2021-05-20T18:16:19Z

@dotnet/roslyn-compiler For the second review.

RikkiGibson · 2021-05-20T21:58:52Z

Thanks for the contribution @Youssef1313!

…ures/interpolated-string * upstream/main: (92 commits) Keep casts when necessary to prefer a constant pattern over a type pattern Remove SyntaxKind.DataKeyword (#53614) Display 'readonly' for record structs (#53634) Update Building, Debugging, and Testing on Windows.md (#53543) Update dependencies from https://github.com/dotnet/arcade build 20210521.3 (#53617) Introduce resx for BuildValidator and MS.CA.Rebuild to allow localization (#53447) Report obsoletion diagnostics for slice and indexer (#53463) Update BasicGenerateConstructorDialog.cs Add searchbox in generate overrides dialog Allow `with` on anonymous types (#53248) Report diagnostic on correct node (#53538) Fix NotNullIfNotNull delegate conversion (#53409) Verify quick info session in InvokeQuickInfo Remove unnecessary retry Ensure no navbar IO on the UI thread Enable nullable reference types Fix timeout behavior in GetQuickInfo Add a semantic model based GetQuickInfoAsync entry point into QuickInfoServiceWithProviders Move semantic model based quick info API up to CommonQuickInfoProvider type Fix dnceng build by forcing the use of xcopy msbuild ...

ghost added the Area-Compilers label May 20, 2021

Youssef1313 closed this May 20, 2021

Youssef1313 reopened this May 20, 2021

Youssef1313 added 2 commits May 20, 2021 13:04

Report diagnostic on correct node

962e833

Fix compiler tests and add IDE test

ae1f004

Youssef1313 marked this pull request as ready for review May 20, 2021 11:12

Youssef1313 requested a review from a team as a code owner May 20, 2021 11:12

Youssef1313 force-pushed the patch-12 branch from 4a5bd4e to ae1f004 Compare May 20, 2021 11:12

Youssef1313 requested a review from a team as a code owner May 20, 2021 11:12

Fix test

9bf6cf1

davidwengier reviewed May 20, 2021

View reviewed changes

davidwengier approved these changes May 20, 2021

View reviewed changes

Fix tests

a1f3222

AlekseyTs added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label May 20, 2021

AlekseyTs reviewed May 20, 2021

View reviewed changes

Youssef1313 added 2 commits May 20, 2021 15:53

Fix test

254f89b

Add test

24e71a6

Youssef1313 added 2 commits May 20, 2021 18:37

Add test

ef1ed5d

Fix test

c4e8ad8

AlekseyTs approved these changes May 20, 2021

View reviewed changes

RikkiGibson self-assigned this May 20, 2021

RikkiGibson approved these changes May 20, 2021

View reviewed changes

RikkiGibson merged commit d513ddf into dotnet:main May 20, 2021

ghost added this to the Next milestone May 20, 2021

Youssef1313 deleted the patch-12 branch May 21, 2021 03:14

RikkiGibson modified the milestones: Next, 17.0.P2 Jun 29, 2021

	VerifyDiagnostics(
	// (7,30): error CS0266: Cannot implicitly convert type 'double' to 'int'. An explicit conversion exists (are you missing a cast?)
	// int[] arr4 = new int[x];// Invalid
	Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "x").WithArguments("double", "int").WithLocation(7, 30),
	// (10,30): error CS0266: Cannot implicitly convert type 'float' to 'int'. An explicit conversion exists (are you missing a cast?)
	// int[] arr5 = new int[y];// Invalid
	Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "y").WithArguments("float", "int").WithLocation(10, 30),
	// (13,30): error CS0266: Cannot implicitly convert type 'decimal' to 'int'. An explicit conversion exists (are you missing a cast?)
	// int[] arr6 = new int[z];// Invalid
	Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "z").WithArguments("decimal", "int").WithLocation(13, 30));

Conversation

Youssef1313 commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidwengier May 20, 2021

Choose a reason for hiding this comment

Uh oh!

Youssef1313 May 20, 2021

Choose a reason for hiding this comment

Uh oh!

davidwengier left a comment

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Youssef1313 May 20, 2021

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 20, 2021

Choose a reason for hiding this comment

Uh oh!

Youssef1313 May 20, 2021

Choose a reason for hiding this comment

Uh oh!

AlekseyTs May 20, 2021

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented May 20, 2021

Uh oh!

AlekseyTs commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs left a comment

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented May 20, 2021

Uh oh!

RikkiGibson commented May 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Youssef1313 commented May 20, 2021 •

edited

Loading

AlekseyTs May 20, 2021 •

edited

Loading

AlekseyTs commented May 20, 2021 •

edited

Loading

AlekseyTs commented May 20, 2021 •

edited

Loading