Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Merge remote-tracking branch 'dotnet/master' into NullableFeature#23560

Merged
tannergooding merged 33 commits intodotnet:NullableFeaturefrom
tannergooding:NullableFeature
Mar 29, 2019
Merged

Merge remote-tracking branch 'dotnet/master' into NullableFeature#23560
tannergooding merged 33 commits intodotnet:NullableFeaturefrom
tannergooding:NullableFeature

Conversation

@tannergooding
Copy link
Copy Markdown
Member

CC. @safern, @krwq, @buyaa-n, @stephentoub

This is primarily for bringing in the new compiler toolset.

VSadov and others added 30 commits March 19, 2019 12:41
Made sure tests actually test something
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
* Update dependencies from https://github.com/dotnet/corefx build 20190328.1

- Microsoft.NETCore.Platforms - 3.0.0-preview4.19178.1
- Microsoft.Private.CoreFx.NETCoreApp - 4.6.0-preview4.19178.1
* Update dependencies from https://github.com/dotnet/arcade build 20190327.11

- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19177.11
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19177.11
* GetPinnableReference on String.cs

* Add attributes for debugger and performance
Resolve 22303   (interlocked failures on ARM64)
These are required by the innerloop flow jobs.
* calendar

* nullable: all calendars

* fix likely corert error
Restore Windows arm32/arm64 innerloop build jobs
* dnceng-linux-internal-temp -> BuildPool.Ubuntu.1604.Amd64
* dotnet-internal-temp -> BuildPool.Windows.10.Amd64.VS2017
* Fix Helix queue names

* Fix open Deb9 queue name.
* [WIP] Struct & SIMD improvements

- Enable CSE of struct values when handle is available (and add code to get the handle of HW SIMD types)
- Don't require block nodes for SIMD assignments
- Don't set `GTF_GLOB_REF` on `GT_OBJ` if it is local
- Set `lvRegStruct` on promoted SIMD fields
- Add tests for #19910 (fixed with this PR) and #3539 & #19438 (fixed with #21314)
- Additional cleanup

Fix #19910
* Fix codegen for StoreNonTemporal

Also, add some asserts and mark some intrinsics as not supporting containment.

Fix #23509
* exclude failing in PRs tests
* Add test for IncrementingPollingCounter

* Fix a bug in Increment calculation in IncrementingPollingCounter

* Remove setting DisplayName property since that's a private property for now

* fix comment

* Remove unused variables
* Delete `setup_coredis_tools` from runtest.py.

* delete dead `setup_coredis_tools` .
* exclude failing in PRs tests

* And another one.

* Exclude for all platforms.
* Update dependencies from https://github.com/dotnet/core-setup build 20190328.01

- Microsoft.NETCore.App - 3.0.0-preview4-27528-01
* Clean `valuenum.cpp`.

* Clean `emitarm64.cpp`.

* Clean `lsraarm64.cpp`.

* Clean `lsraarmarch.cpp`.

* Clean `lowerarmarch.cpp`.

* Clean `lower.cpp`.

* Clean `ssabuilder.cpp`.

* Clean `simd.cpp`.

* Clear `simdcodegenxarch.cpp`.

* Clean `lowerxarch.cpp`.

* Clean `scopeinfo.cpp`.
…, respectively (#23536)

* Removing FeedTasksPackageVersion from dependencies.props
* The corresponding metadata was removed in #22884

* Updating BuildTools, CoreCLR to preview4-03828-01, preview4-27528-71, respectively

* Adding a ! in String.Searching.cs
…329.1 (#23554)

- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19179.1
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19179.1
@safern
Copy link
Copy Markdown
Member

safern commented Mar 29, 2019

Should we instead rebase the branch? Will the merge commit show when we submit a PR to merge in master?

@tannergooding
Copy link
Copy Markdown
Member Author

tannergooding commented Mar 29, 2019

Will the merge commit show when we submit a PR to merge in master?

Ultimately, yes. But that is also good as it shows how the branches were integrated together. This is just normal git workflow and is what almost every other repo in existence does.

For a closer to home comparison; Roslyn has multiple feature branches (which is what this branch effectively is) and has infrastructure setup to automatically flow the changes through between branches (release branches flow into master and master flows into feature branches).

They can end up doing multiple merge commits between branches a day (although this is, more often than not, less frequent for feature branches) and don't have any problems with the workflow.

@safern
Copy link
Copy Markdown
Member

safern commented Mar 29, 2019

@krwq it looks like the new compiler is complaining about the workaround for field initialization:

shared\System\Globalization\StringInfo.cs(29,13): error CS1717: Assignment made to same variable; did you mean to assign something else? [D:\a\1\s\src\System.Private.CoreLib\System.Private.CoreLib.csproj]

https://github.com/dotnet/coreclr/blob/NullableFeature/src/System.Private.CoreLib/shared/System/Globalization/StringInfo.cs#L29

I guess we should disable this warning as well?

@tannergooding
Copy link
Copy Markdown
Member Author

tannergooding commented Mar 29, 2019

It should also be noted: DO NOT Squash and merge this or Rebase and merge this PR.

This PR must be merged with Create a merge commit for everything to work cleanly and to not result in further merge conflicts, duplicated history, etc later down the line.

Comment thread src/System.Private.CoreLib/shared/System/Globalization/StringInfo.cs Outdated
@krwq
Copy link
Copy Markdown
Member

krwq commented Mar 29, 2019

@tannergooding since this is not fixing any of the issues we have found and breaks workarounds, does it make sense to merge this right now? Perhaps we should wait for more fixes or at least better way to workaround the issues - I feel like in-lining code is more error-prone than the current workaround

@tannergooding
Copy link
Copy Markdown
Member Author

@krwq, yes it makes sense. This brings in the new compiler bits which includes the updated analysis rules and which will allow fixing several of the workarounds that have been made.

The actualy workarounds should be removed independently, however, and this is just touching up a single place where the build would otherwise fail. I've also updated this to no longer inline and to explicitly do _str = null! instead, which was indicated to be the correct resolution for these cases.

@safern
Copy link
Copy Markdown
Member

safern commented Mar 29, 2019

Once this PR is merged, I'll run the build that creates the PR on master 😄


public StringInfo(string value)
{
_str = null!;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

actually, not too bad and easier to grep for

@tannergooding tannergooding merged commit 427bedd into dotnet:NullableFeature Mar 29, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…ature

Merge remote-tracking branch 'dotnet/master' into NullableFeature

Commit migrated from dotnet/coreclr@427bedd
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.