Skip to content

C#: upgrade Roslyn dependencies to version 3.7#4041

Merged
tamasvajk merged 8 commits intogithub:mainfrom
tamasvajk:feature/update-roslyn
Sep 14, 2020
Merged

C#: upgrade Roslyn dependencies to version 3.7#4041
tamasvajk merged 8 commits intogithub:mainfrom
tamasvajk:feature/update-roslyn

Conversation

@tamasvajk
Copy link
Contributor

No description provided.

@tamasvajk tamasvajk requested a review from a team August 11, 2020 11:10
@adityasharad adityasharad changed the base branch from master to main August 14, 2020 18:33
@tamasvajk tamasvajk force-pushed the feature/update-roslyn branch from 5e5224b to fa3e038 Compare September 4, 2020 09:59
@github-actions github-actions bot added the C# label Sep 4, 2020
@tamasvajk
Copy link
Contributor Author

I think this is expected to fail, because the tests can't build a new codeql CLI without the new dependencies.

case TypeKind.Error:
var named = (INamedTypeSymbol)type;
if (named.IsTupleType)
if (named.IsTupleType && named.TupleUnderlyingType is object)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tuple<T1,T2,T3,T4,T5,T6,T7,TRest> and similar are IsTupleType == true, but TupleUnderlyingType == null


var underlyingType = NamedType.Create(Context, symbol.TupleUnderlyingType);
trapFile.tuple_underlying_type(this, underlyingType);
if (symbol.TupleUnderlyingType is object)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might be null see, below.

trapFile.tuple_underlying_type(this, underlyingType);
if (symbol.TupleUnderlyingType is object)
{
var underlyingType = TupleType.Create(Context, symbol.TupleUnderlyingType);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TupleUnderlyingType is not a NamedType, but a TupleType.

@tamasvajk
Copy link
Contributor Author

tamasvajk commented Sep 4, 2020

With these changes the extracted data changed too. I found the following

#  | [InstanceConstructor] ValueTuple
#--|   2: (Parameters)

in the print ast expected files.

@hvitved Didn't you fix something similar during the assembly prefix PR?

@tamasvajk tamasvajk force-pushed the feature/update-roslyn branch 5 times, most recently from 7d2bde8 to 092c0a0 Compare September 8, 2020 09:16
tamasvajk and others added 4 commits September 10, 2020 13:53
The documentation on `ExplicitInterfaceImplementations` says "Properties
imported from metadata can explicitly implement more than one property", so
the constraint appears to be invalid.
@hvitved hvitved force-pushed the feature/update-roslyn branch from 092c0a0 to de7f850 Compare September 10, 2020 12:11

explicitly_implements(
unique int id: @member ref,
int id: @member ref,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the case covered by the quoted comment. Is it the following:

using System;

interface IA
{
	int Prop
	{
		get;
	}
}

interface IB : IA
{
}

class C : IB
{
	int IA.Prop // Does this property explicitly implement both IA and IB?
	{
		get
		{
			return 1;
		}
	}

	public int Prop
	{
		get
		{
			return 2;
		}
	}
}

public class Program
{
	public static void Main()
	{
		var c = new C();
		Console.WriteLine(c.Prop); // 2
		Console.WriteLine(((IA)c).Prop); // 1
		Console.WriteLine(((IB)c).Prop); // 1
	}
}

@hvitved hvitved force-pushed the feature/update-roslyn branch from de7f850 to 6c5b30d Compare September 11, 2020 09:55
@tamasvajk tamasvajk merged commit d21c101 into github:main Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants