Runtime: Remove Dictionary<K,V>.GetValueOrDefault instance methods and IDictionary<K,V>.GetValueOrDefault extension methods

Created on 5 Apr 2017  路  69Comments  路  Source: dotnet/runtime

IDictionary<TKey,TValue> already has:

c# partial class CollectionExtensions { public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key); public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue); }

As extension methods in CollectionExtensions.

In this discussion: dotnet/runtime#20684 we've decided to remove this instance methods, see: https://github.com/dotnet/corefx/issues/17275#issuecomment-291636863

cc: @danmosemsft @ianhays

api-approved area-System.Collections easy up-for-grabs

Most helpful comment

Fixed with 3 PRs: dotnet/corefx#19247, https://github.com/dotnet/corert/pull/3515, https://github.com/dotnet/coreclr/pull/11388

Thanks @Pothulapati for your contribution.

All 69 comments

This is fairly easy, while important for 2.0 and interesting as it touches both CoreFX and CoreCLR:

Next steps:

  1. Undo parts of https://github.com/dotnet/corefx/pull/16605

    • Delete APIs from ref contract: source code

    • Update uapaot baseline by running msbuild src\System.Collections\src\System.Collections.csproj /p:TargetGroup=uapaot /p:BaselineAllApiCompatError=true

  2. When that is checked in, undo also https://github.com/dotnet/coreclr/pull/8641

    • Don't do it before [1], otherwise you will block continuous updates of CoreCLR new bits into CoreFX

  3. Remove implementation from CoreRT as well. Code to remove

    • Don't do it before [1], otherwise you will block continuous updates of CoreRT new bits into CoreFX -- this can be done at the same time as [2]

I'm already in progress :)

Ah, pity, would be great 'easy' issue - need a couple for new contributors ...

Give this one to them then :) So that they can get in context with the code -- I was just taking it to clean up my area as fast as possible. Unassigned!!

Edited your next step comment to add a little bit more detailed for new contributors.

I will Grab this, It's just deleting ref's and some code, Nothing to Code. Right?

Hi @Pothulapati thanks for willing to contribute. You'll need to accept an invitation to collaborate so that we can assign it to you. I will assign it to myself meanwhile but now this issue is yours :) feel free to start working on it.

How do we send the invitation? @karelz @danmosemsft

Invitation sent - ping us when you accept @Pothulapati.

Thanks @safern, I appreciate you giving it up for the greater good ;-)

Accepted The Invitation. Looking forward to the Issue.

@Pothulapati the issue is assigned to you. You can start working on it!

If you have any questions please feel free to ask them here. Hope you enjoy contributing!

Thanks @karelz

@safern Forked The Repo, When I try to Build The Repo For the first time, The "Installing dotnet Cli..." is taking a long time and not showing anything. It's because of the slow download speeds of the dotnet cli. Is There any alternative way to install dotnet cli?

Hi, @Pothulapati did you get it to download? It shouldn't take that long. My guess would be more of like a network speed problem on your side. Before calling build.cmd on your repo you could try init-tools.cmd which is the standalone command to install cli.

No,Network speed on my side is fast. I also Tried init-tools.cmd . It's the same. Even after 5 hours of Installing Dotnet cli... ,It doesn't respond.

5 hours is definitely not what it would take even with a slow network. I suspect something went wrong. Can you try these two things to try to get us more info on what is going on:

  • There should be a init-tools.log that gets generated in the process, and that will most likely give us more info that will help diagnosing the issue.
  • Can you try running set _echo=on before running build.cmd? that will give us a more verbose output and tell us what might be going on and which step is the one that is stuck.

@joperezr Now If I Run init-tools.cmd I get the following
C:\Users\Tarun\Source\Repos\corefx\Tools\DOTNET~1\dotnet.tar - The process cannot access the file because it is being used by another process. Installing dotnet cli...
and the log file as
Running "C:\Users\Tarun\Source\Repos\corefx\init-tools.cmd" Installing 'https://dotnetcli.blob.core.windows.net/dotnet/preview/Binaries/1.0.0-preview2-1-003182/dotnet-dev-win-x64.1.0.0-preview2-1-003182.zip' to 'C:\Users\Tarun\Source\Repos\corefx\Tools\dotnetcli\dotnet-dev-win-x64.1.0.0-preview2-1-003182.zip'

looks like some process is still getting a handle to that zip, is there a way for you to try find that process and kill it, and then clean your repo: clean.cmd -all

@joperezr Now,That I have found the process and Killed it. It Just Shows Installing dotnet cli.. but still the download is taking a lot of time. Why don't you try Downloading the dotnet-dev-win-x64.1.0.0-preview2-1-003182.zip so as to make sure it's not a network problem from my side?

@Pothulapati I'm able to download -- where you able to figure it out? After you kill the process and cleaned your repo what does the log file says?

@jkotas this should be done in CoreRT as well right?

Yes - since Dictionary was not moved to the shared corelib partition yet.

Thanks @jkotas

I updated the instructions to follow in https://github.com/dotnet/corefx/issues/17917#issuecomment-291749382 so that you don't forget @Pothulapati

@Pothulapati sorry to hear about your troubles with getting started :( ... this is truly weird and exceptional. We have plenty of contributors and this is the first time we hit similar issue.

Please if you, can try another machine, VM, new directory, or anything you can think of -- there is something truly weird going on on your machine and I would like to first get you unblocked ASAP, and then deep dive into what happened and why, so that we can avoid such bad experience in future for new contributors. Thank you for you patience and sorry for that!
Note: This is definitely not our typical first-time contributor experience.

@safern After killing the process and running the clean.cmd I still get "Installing dotnet cli..." in the clean cmd(and it doesn't respond too).Sorry For Bugging you.

@karelz I tried in another machine but it shows the same thing but when I run it in an Azure VM, It Works Fine.

If you can share what your init-tools.log shows we should be able to better diagnose what is going on.

@Pothulapati happy to hear you are unblocked. However, I would still like to dig into the infrastructure issue in parallel to find out what's wrong on your machines - please follow @joperezr's instructions if you can. We may want to take it into separate issue (infra issue) - can you please file a new one and link it here? Thanks! (cc @mellinoe)

@karelz @joperezr @safern I Overcame the init-tools error by resetting my PC. Now , When I try to build the project I get the following error even after doing clean.cmd

Tools are already initialized.
Running: C:\Users\tarun\Source\Repos\corefx\src\Native\build-native.cmd   x64  Debug  Windows_NT  --numproc 4    toolSetDir=c:\tools\clr
Tools are already initialized.
Running: C:\Users\tarun\Source\Repos\corefx\Tools\msbuild.cmd /nologo /verbosity:minimal /clp:Summary /maxcpucount /nodeReuse:false /l:BinClashLogger,Tools\net46\Microsoft.DotNet.Build.Tasks.dll;LogFile=binclash.log /p:ConfigurationGroup=Debug /p:BuildPackages=false /p:GenerateNativeVersionInfo=true  /flp:v=normal  /flp2:warningsonly;logfile=msbuild.wrn  /flp3:errorsonly;logfile=msbuild.err  C:\Users\tarun\Source\Repos\corefx\src\Native\..\../build.proj /t:GenerateVersionHeader

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.53
Command execution succeeded.
**********************************************************************
** Visual Studio 2017 Developer Command Prompt v15.0.26403.0
** Copyright (c) 2017 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x86_x64'
Commencing build of native components

-- The C compiler identification is MSVC 19.0.24218.2
-- The CXX compiler identification is MSVC 19.0.24218.2
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/Shared/14.0/VC/bin/x86_amd64/cl.exe
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/Shared/14.0/VC/bin/x86_amd64/cl.exe -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/Shared/14.0/VC/bin/x86_amd64/cl.exe
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/Shared/14.0/VC/bin/x86_amd64/cl.exe -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Failed
-- Performing Test COMPILER_HAS_DEPRECATED
-- Performing Test COMPILER_HAS_DEPRECATED - Success
-- Configuring done
-- Generating done
-- Build files have been written to: C:/Users/tarun/Source/Repos/corefx/bin/obj/Windows_NT.x64.Debug/native
Tools are already initialized.
The specified config file 'C:\Users\tarun\Source\config.json' does not exist.
Error: Could not load Json configuration file.
Failed to generate native component build project!
Command execution failed with exit code 1.

As per my understanding,It is searching for the config.json File in the Source Folder(Where the corefx repo is present) but not inside corefx(Where the config.json is present).

Even I place config.json file in the Source folder,I get the following error

Tools are already initialized.
Running: C:\Users\tarun\Source\Repos\corefx\src\Native\build-native.cmd   x64  Debug  Windows_NT  --numproc 4    toolSetDir=c:\tools\clr
Tools are already initialized.
Running: C:\Users\tarun\Source\Repos\corefx\Tools\msbuild.cmd /nologo /verbosity:minimal /clp:Summary /maxcpucount /nodeReuse:false /l:BinClashLogger,Tools\net46\Microsoft.DotNet.Build.Tasks.dll;LogFile=binclash.log /p:ConfigurationGroup=Debug /p:BuildPackages=false /p:GenerateNativeVersionInfo=true  /flp:v=normal  /flp2:warningsonly;logfile=msbuild.wrn  /flp3:errorsonly;logfile=msbuild.err  C:\Users\tarun\Source\Repos\corefx\src\Native\..\../build.proj /t:GenerateVersionHeader

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.50
Command execution succeeded.
**********************************************************************
** Visual Studio 2017 Developer Command Prompt v15.0.26403.0
** Copyright (c) 2017 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x86_x64'
Commencing build of native components

-- The C compiler identification is MSVC 19.0.24218.2
-- The CXX compiler identification is MSVC 19.0.24218.2
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/Shared/14.0/VC/bin/x86_amd64/cl.exe
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/Shared/14.0/VC/bin/x86_amd64/cl.exe -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/Shared/14.0/VC/bin/x86_amd64/cl.exe
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/Shared/14.0/VC/bin/x86_amd64/cl.exe -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Failed
-- Performing Test COMPILER_HAS_DEPRECATED
-- Performing Test COMPILER_HAS_DEPRECATED - Success
-- Configuring done
-- Generating done
-- Build files have been written to: C:/Users/tarun/Source/Repos/corefx/bin/obj/Windows_NT.x64.Debug/native
Tools are already initialized.
Running: C:\Users\tarun\Source\Tools\msbuild.cmd /nologo /verbosity:minimal /clp:Summary /maxcpucount /nodeReuse:false /l:BinClashLogger,Tools\net46\Microsoft.DotNet.Build.Tasks.dll;LogFile=binclash.log /p:ConfigurationGroup=Debug /p:BuildPackages=false  /flp:v=normal  /flp2:warningsonly;logfile=msbuild.wrn  /flp3:errorsonly;logfile=msbuild.err  C:/Users/tarun/Source/Repos/corefx/src/Native/../../bin/obj/Windows_NT.x64.Debug/native\install.vcxproj /t:rebuild /p:Configuration=Debug /p:Platform=x64 /p:PlatformToolset=v141
Error in the command: C:\Users\tarun\Source\Tools\msbuild.cmd /nologo /verbosity:minimal /clp:Summary /maxcpucount /nodeReuse:false /l:BinClashLogger,Tools\net46\Microsoft.DotNet.Build.Tasks.dll;LogFile=binclash.log /p:ConfigurationGroup=Debug /p:BuildPackages=false  /flp:v=normal  /flp2:warningsonly;logfile=msbuild.wrn  /flp3:errorsonly;logfile=msbuild.err  C:/Users/tarun/Source/Repos/corefx/src/Native/../../bin/obj/Windows_NT.x64.Debug/native\install.vcxproj /t:rebuild /p:Configuration=Debug /p:Platform=x64 /p:PlatformToolset=v141. => The system cannot find the file specified
Command execution failed with exit code 1.
Failed to generate native component build project!
Command execution failed with exit code 1.

I see that it is searching for the msbuild.cmd in the Source\Tools\msbuild.cmd but the msbuild.cmd is present inside Source\repos\corefx\Tools\

I was able to fix the above said issue by copying both the config.json and Tools Folder to the Source Folder but Why is it searching in the Source Folder rather than Sourcereposcorefx
This is should also be fixed as the building process would be smooth then 馃憤
Now,Working on the Issue 馃拑

@safern I successfully completed the 1st step i.e removing Code from the ref contract and successfully ran the command. Now I need to create a PR in the corefx Repo? and once the PR is merged then I have to Work on the coreclr,Corert repos and submit PR's on the respective repos.Does that Sound Fine?

I was able to fix the above said issue by copying both the config.json and Tools Folder to the Source Folder but Why is it searching in the Source Folder rather than Sourcereposcorefx

I believe that the problem here is that you have different enlistments on the same folder path, as we don't even have a folder called \repos\corefx. I believe that in order for you to go to a clean state, try to create a new folder entirely(could be something in your desktop, like C:\users\tarun\Desktop\repos) and then try doing a git clone to corefx from there so that you are in an isolated repo. After that, you should be able to just run build.cmd and that should just work. You should never need to copy files like config.json or msbuild.cmd around.

@safern In the comment about the Removal of Code in CoreRT,The Reference to the code that is to be removed is wrong. The methods GetValueOrDefault are to be removed right?

@Pothulapati you are correct, I accidentally pasted the link to 'master' which changed since then -- I updated the link above to the particular source code version - the task is to remove both GetValueOrDefault methods.

@karelz And In the Coreclr thing,I was asked to UNDO the changes rather than to remove the Methods.So,That means internal TValue GetValueOrDefault(TKey key) method needs to stay?

@Pothulapati yeah, undo is probably better from history point of view (still new to git myself).
What else needs the internal method in CoreRT? Maybe we need to keep it there from that reason ...

undo is probably better from history point of view (still new to git myself)

I don't know what "undo" means in this context, but if you mean rewrite the history of the branch to not include those changes, no, we should not do that. Except in super drastic circumstances, once something is merged, it's there in the history forever, and to remove something you submit a new commit to do so.

Based on the replies above, I just assume there is difference between undo and edit. If there's not, I totally agree - in which case I wonder what @Pothulapati's reference meant (I don't see the CoreCLR link). Maybe it just meant "reverse the previous change, don't do more fancy stuff"?

@karelz I will reverse the changes by doing a commit but my exact concern is whether the internal TValue GetValueOrDefault(TKey key) should stay or not?

@Pothulapati can you do the research what else needs the internal method in CoreRT? (one way is to remove it and run build & tests to see if something fails, or search the code base ...)
It would be overall best to ask that question on CoreRT repo (e.g. in your PR) - @MichalStrehovsky and @jkotas will be able to provide guidance there.

what else needs the internal method in CoreRT?

Nothing needs in CoreRT.

@Pothulapati any update on the pending work needed to close this issue?

I understood that GetValueOrDefault extension methods on IDictionary and IReadOnlyDictionary don't play well together unless there is a disambiguating GetValueOrDefault specific to Dictionary, because otherwise the compiler will complain that the extension method is ambiguous.

@jnm2 @safern do we plan to add the IReadOnlyDictionary extensions? I don't see them in the top post ...
If we plan both, then adding Dictionary extension as well may make sense. We should run it by API review in such case.

@karelz now that i'm going through the PR (#16605) that added IDictionary extensions we added IReadOnlyDictionary extensions as well but in that time frame Dictionary had instance methods so it we wouldn't have that ambiguity when using Dictionary. So we should definitely add Dictionary<TKey, TValue>.GetValueOrDefault extension methods as if not we will cause our customers this ambiguity and will face compilation errors.

Thanks @jnm2 for pointing this out, I completely missed it.

@dotnet/fxdc quick review ask: We are pulling out Dictionary.GetValueOrDefault, but we need to delete an extension method to avoid compile-time ambiguity between IDictionary.GetValueOrDefault and IReadOnlyDictionary.GetValueOrDefault. Can we get quick ack on the plan please?

First proposed API (Remove IReadOnlyDictionary extension method)

partial class CollectionExtensions
{
  public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue value)
  public static bool Remove<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, out TValue value)
  public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key);
  public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue);
- public static TValue GetValueOrDefault<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary, TKey key);
- public static TValue GetValueOrDefault<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue);
}

Keep IDictionary<TKey, TValue> extension method as it is implemented by more types and it is used in more places.

Second Option

Delete all GetValueOrDefault<TKey, TValue> extension methods because first of all it is not a very used api, and it solves a very simple problem. This would give our costumers the option to implement their own extension methods for any interface they want without causing ambiguities.

partial class CollectionExtensions
{
  public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue value)
  public static bool Remove<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, out TValue value)
- public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key);
- public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue);
- public static TValue GetValueOrDefault<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary, TKey key);
- public static TValue GetValueOrDefault<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue);
}

See details: in today's code

If we have to API reviewers' upvotes, let's go ahead. If there are questions, let's bring it to proper API review on Tuesday.

Thanks @karelz for bringing that quick API Proposal, hopefully we can get an answer soon so that we can solve the ambiguity.

While we are here, people will run into the exact same situation with all the classes in the framework which implement IReadOnlyDictionary<,> because they all happen to implement IDictionary<,> as well:

@karelz did we get any update from the review group?

This seems to be a breaking change we would be introducing, it can be workaround from our costumers but not ideal.

cc: @terrajobst

Our having added this extension to both IDictionary and IReadOnlyDictionary causes a problem for any type that implements both interfaces, something we expect to be common. We shouldn't add a specific overload for every one of our own types that implements both, and we can't do so for types we don't own (any 3rd party collection that implements both). I suggest the right solution is to remove either the extensions on IDictionary or the extensions on IReadOnlyDictionary, leaving just one of them.

I'd hope to encourage wider implementation of IReadOnlyDictionary rather than IDictionary.

I vaguely remember the discussions about IDictionary usage vs. IReadOnlyDictionary - was it on the original bug?

We didn't review it, because I didn't mark it for API review :( ... we may need to postpone it for next Tuesday.

@karelz Are you referring to this comment: https://github.com/dotnet/corefx/issues/3482#issuecomment-263007579?

Yep, thanks @svick! I thought it was you doing the research, just wasn't 100% sure. And kudos for monitoring entire CoreFX (I don't understand how you handle the volume :)).

So then we should mark this as api-ready-for-review and update the proposal to be deleting one of the extensions methods.

I guess something like that ...

I've updated the API proposal, so I'm marking it ready for review: https://github.com/dotnet/corefx/issues/17917#issuecomment-296093667

We believe the best course is to remove the overloads that take IDictionary<K,V> and leave IReadOnlyDictionary<K,V>. Rationale:

  • The important BCL types implementing IDictionary<K,V> also implement IReadOnlyDictionary<K,V>
  • It seems likely that we'll be able to change the inheritance hierarchy to make IDictionary<K,V> extend IReadOnlyDictionary<K,V>
partial class CollectionExtensions
{
  public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue value)
  public static bool Remove<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, out TValue value)
- public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key);
- public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue);
  public static TValue GetValueOrDefault<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary, TKey key);
  public static TValue GetValueOrDefault<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary, TKey key, TValue defaultValue);
}

@Pothulapati could you please follow up with this issue? Next steps:

  1. Remove IDictionary<TKey, TValue>.GetValueOrDefault() extension methods from CollectionExtensions.cs: https://github.com/dotnet/corefx/blob/master/src/System.Collections/src/System/Collections/Generic/CollectionExtensions.cs#L9-#L23
  1. Remove APIs from ref contract: https://github.com/dotnet/corefx/blob/master/src/System.Collections/ref/System.Collections.cs#L47-L49

  2. Remove specific tests for this APIs:
    https://github.com/dotnet/corefx/blob/master/src/System.Collections/tests/Generic/CollectionExtensionsTests.cs#L12-L43

Could you do this on a PR please?

P.S: You are still missing to remove Dictionary<TKey, TValue>.GetValueOrDefault() implementation methods from coreCLR (remove this) and coreRT (remove this).

@safern In The CoreClr Repo,If I remove the GetValueOrDefault Methods from the Dictionary,I Get The Following Error
4>src\System\RtType.cs(1203,46): error CS1061: 'Dictionary<string, RuntimeEventInfo>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<string, RuntimeEventInfo>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\RtType.cs(1359,83): error CS1061: 'Dictionary<string, List<RuntimePropertyInfo>>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<string, List<RuntimePropertyInfo>>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2043,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2081,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2110,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2143,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2173,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2204,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2234,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2286,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2312,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2337,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>src\System\Reflection\CustomAttribute.cs(2360,31): error CS1061: 'Dictionary<RuntimeType, RuntimeType>' does not contain a definition for 'GetValueOrDefault' and no extension method 'GetValueOrDefault' accepting a first argument of type 'Dictionary<RuntimeType, RuntimeType>' could be found (are you missing a using directive or an assembly reference?) [D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj] 4>Done Building Project "D:\code\coreclr\src\mscorlib\System.Private.CoreLib.csproj" (Build target(s)) -- FAILED. 2>Done Building Project "D:\code\coreclr\src\build.proj" (Build target(s)) -- FAILED. 1>Done Building Project "D:\code\coreclr\build.proj" (default targets) -- FAILED.

It's Because src\System\Reflection\CustomAttributes.cs & src\System\RtType.cs uses the Dictionary.GetValueOrDefault Methods.

(@Pothulapati That would be easier to read if fenced with ``` ... ``` on new lines.)

@safern Sorry,I mentioned that the error is from CoreRt repo(edited the comment now),The error is from the Coreclr repo.
@jnm2 Oh Yes.I was thinking the same but didn't know how to change .Thanks,:D

Maybe we should move CollectionExtensions implementation to S.Private.Corelib because it seems like CoreRT and CoreCLR implementations use Dictionary<TKey, TValue>.GetValueOrDefault()

cc: @stephentoub @jkotas @ianhays

I would not move CollectionExtensions down. We should just update them to not use this helper.

@Pothulapati could you just update CostumAttribute and RtType to not use Dictionary<TKey, TValue>.GetValueOrDefault on your PR?

not use Dictionary.GetValueOrDefault on your PR?

It should be replaced by ContainsKey in most places - it should result in tiny bit faster and smaller code.

Fixed with 3 PRs: dotnet/corefx#19247, https://github.com/dotnet/corert/pull/3515, https://github.com/dotnet/coreclr/pull/11388

Thanks @Pothulapati for your contribution.

Thank you @Pothulapati!

@karelz @safern @jkotas Great Learning experience for me.Never expected the dotnet community to be this friendly and helping.Thank You Very Much.Looking forward to contribute more.

Very happy to hear you liked it @Pothulapati.

If you're interested, please help us make the new contributor docs better here: https://github.com/dotnet/corefx/wiki/New-contributor-Docs#main-page-section-content (writeable by anyone for easier collaboration, hacking on it with @haralabidis for last week)

You might also find this useful & interesting: http://rion.io/2017/04/28/contributing-to-net-for-dummies/

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Timovzl picture Timovzl  路  3Comments

sahithreddyk picture sahithreddyk  路  3Comments

btecu picture btecu  路  3Comments

jkotas picture jkotas  路  3Comments

chunseoklee picture chunseoklee  路  3Comments