Mirror changes from dotnet/corefx - #15909
Conversation
* Consolidate System.Memory code to shared folder This change is removing the duplicate codes from System.Memory and keep only one copy under the shared folder to be easier to edit such code in one place and get reflected on the other repos. * Address the review feedback * Addressing more feedback * More cleanup * remove empty line and added a comment Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
|
@dotnet-bot test Tizen armel Cross Checked Innerloop Build and Test please |
| if (IsDisposed) | ||
| { | ||
| ThrowHelper.ThrowObjectDisposedException(nameof(OwnedMemory<T>), ExceptionResource.Memory_ThrowIfDisposed); | ||
| ThrowHelper.ThrowObjectDisposedException_MemoryDisposed(nameof(OwnedMemory<T>)); |
There was a problem hiding this comment.
Bit odd to have a unique exception handler; then add a chunky nameof(OwnedMemory<T>) param?
There was a problem hiding this comment.
In general, I think we'll need to do some clean up for the exceptions and using ThrowHelper. I hope at some point we'll have a shared ThrowHelper across repos too and revisit what parameters every helper method is using.
There was a problem hiding this comment.
we'll have a shared ThrowHelper across repos too
The eventual solution should be to have IL rewriter or some similar tech to do this optimization for you. ThrowHelper is a pain to maintain. Until we have automated way to do this, we want to use only in the places that really matter. We do not want to share accross repos.
There was a problem hiding this comment.
@tarekgh I think @benaadams has valid point. We should move the chunky name into the throw helper and not pass it as an argument. You can append a commit to this PR to fix it - the mirror will then mirror it back to CoreFX.
|
test Tizen armel Cross Checked Innerloop Build and Test |
* Consolidate System.Memory code to shared folder (dotnet/corefx#26393) * Consolidate System.Memory code to shared folder This change is removing the duplicate codes from System.Memory and keep only one copy under the shared folder to be easier to edit such code in one place and get reflected on the other repos. * Address the review feedback * Addressing more feedback * More cleanup * remove empty line and added a comment Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com> * Add missing throw helper methods used in the code we got from corefx * Update the exception helper * fix the break Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
|
@safern I forgot to rebase when merging. instead, I did squash it. is this going to cause a problem for the mirroring? sorry about that. I can revert and resubmit if needed. |
| <data name="Marshaler_StringTooLong" xml:space="preserve"> | ||
| <value>Marshaler restriction: Excessively long string.</value> | ||
| </data> | ||
| <data name="MemoryDisposed" xml:space="preserve"> |
There was a problem hiding this comment.
This string is redundant with Memory_ThrowIfDisposed.
| InvalidOperation_HandleIsNotInitialized, | ||
| AsyncMethodBuilder_InstanceNotInitialized, | ||
| ArgumentNull_SafeHandle, | ||
| MemoryDisposed, |
There was a problem hiding this comment.
This is redundant with Memory_ThrowIfDisposed field
There was a problem hiding this comment.
I'll submit a PR to fix this. thanks for the catch
* Consolidate System.Memory code to shared folder (dotnet/corefx#26393) * Consolidate System.Memory code to shared folder This change is removing the duplicate codes from System.Memory and keep only one copy under the shared folder to be easier to edit such code in one place and get reflected on the other repos. * Address the review feedback * Addressing more feedback * More cleanup * remove empty line and added a comment Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com> * Add missing throw helper methods used in the code we got from corefx * Update the exception helper * fix the break Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
|
Yes it will cause the mirror to get stuck, because of the commit created will not have the signature and will try to mirror it back and there will me conflicts when trying to do so. I will fix the mirror though, no worries :) |
|
@safern let me know when you fix the mirroring as I am expecting we'll need to do a small change in corefx to let the code exactly match. |
|
@tarekgh fixed! |
5 participants
This PR contains mirrored changes from dotnet/corefx
Please REBASE this PR when merging