Update source-gen APIs according to review #59042

layomia · 2021-09-13T17:02:14Z

Implements API approved in #45448 (comment). These are cosmetic changes to APIs called by generated code to configure type metadata, to make them easier to extend with new features in future versions of the generator/STJ.

Should be ported to 6.0.

dotnet-issue-labeler · 2021-09-13T17:02:21Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

ghost · 2021-09-13T17:02:23Z

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

Implements API approved in #45448 (comment). These are cosmetic changes to APIs called by generated code to configure type metadata, to make them easier to extend with new features in future versions of the generator/STJ.

Should be ported to 6.0.

Author:	layomia
Assignees:	layomia
Labels:	`area-System.Text.Json`
Milestone:	6.0.0

src/libraries/System.Text.Json/ref/System.Text.Json.cs

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

src/libraries/System.Text.Json/ref/System.Text.Json.cs

layomia · 2021-09-13T21:42:53Z

I want to call out new commit 4ce31e9 ("Rename fast-path func name and add src-gen/JsonNode interop support"). It implements the proposed API for tomorrow - #45448 (comment).

...em.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/NodeInteropTests.cs

...braries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs

steveharter

LGTM pending API approval especially around the SerializeHandler\Serialize changes.

steveharter · 2021-09-14T14:31:14Z

Possible Mono\WASM issues:

/datadisks/disk1/work/AD8D0967/p/build/wasm/WasmApp.Native.targets(293,5): error : Failed to compile /datadisks/disk1/work/AD8D0967/w/A24F0912/e/wasm_build/obj/wasm/System.Text.Json.SourceGeneration.Tests.dll.bc -> /datadisks/disk1/work/AD8D0967/w/A24F0912/e/wasm_build/obj/wasm/System.Text.Json.SourceGeneration.Tests.dll.o [/datadisks/disk1/work/AD8D0967/w/A24F0912/e/publish/ProxyProjectForAOTOnHelix.proj]

layomia · 2021-09-14T18:52:30Z

LGTM pending API approval especially around the SerializeHandler\Serialize changes.

API has been approved, so this is good to merge when green - #45448 (comment).

eerhardt · 2021-09-16T19:07:55Z

@lewing @radical @SamMonoRT - Have any of you seen the failure before? It appears to be an issue with WASM AOT.

https://helix.dot.net/api/2019-06-17/jobs/770ad140-31bb-4858-9396-d50162076efd/workitems/System.Text.Json.SourceGeneration.Roslyn3.11.Tests/console

  [39/47] xunit.runner.utility.netcoreapp10.dll.bc -> xunit.runner.utility.netcoreapp10.dll.o [took 20.326s]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error : Failed to compile /datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/wasm_build/obj/wasm/System.Text.Json.SourceGeneration.Roslyn3.11.Tests.dll.bc -> /datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/wasm_build/obj/wasm/System.Text.Json.SourceGeneration.Roslyn3.11.Tests.dll.o [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error :  "/datadisks/disk1/work/9D6C08B7/p/build/emsdk/upstream/bin/clang++" -target wasm32-unknown-emscripten -DEMSCRIPTEN -fno-inline-functions -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-cxx-exceptions -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr -D__EMSCRIPTEN_major__=2 -D__EMSCRIPTEN_minor__=0 -D__EMSCRIPTEN_tiny__=23 -D_LIBCPP_ABI_VERSION=2 -Dunix -D__unix -D__unix__ -Werror=implicit-function-declaration -Xclang -iwithsysroot/include/SDL --sysroot=/datadisks/disk1/work/9D6C08B7/p/build/emsdk/upstream/emscripten/cache/sysroot -Xclang -iwithsysroot/include/compat -Oz -g -v -c /datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/wasm_build/obj/wasm/System.Text.Json.SourceGeneration.Roslyn3.11.Tests.dll.bc -o /tmp/tmpkKeaiw.tmp [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error : clang version 13.0.0 (/b/s/w/ir/cache/git/chromium.googlesource.com-external-github.com-llvm-llvm--project 5852582532b3eb3ea8da51a1e272d8d017bd36c9) [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error : Target: wasm32-unknown-emscripten [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error : Thread model: posix [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error : InstalledDir: /datadisks/disk1/work/9D6C08B7/p/build/emsdk/upstream/bin [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error :  (in-process) [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error :  "/datadisks/disk1/work/9D6C08B7/p/build/emsdk/upstream/bin/clang++" -cc1 -triple wasm32-unknown-emscripten -emit-obj --mrelax-relocations -disable-free -main-file-name System.Text.Json.SourceGeneration.Roslyn3.11.Tests.dll.bc -mrelocation-model static -mframe-pointer=none -fno-rounding-math -mconstructor-aliases -target-cpu generic -fvisibility hidden -debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb -v -fcoverage-compilation-dir=/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish -resource-dir /datadisks/disk1/work/9D6C08B7/p/build/emsdk/upstream/lib/clang/13.0.0 -Oz -Werror=implicit-function-declaration -fdebug-compilation-dir=/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish -ferror-limit 19 -fgnuc-version=4.2.1 -fno-inline-functions -vectorize-slp -iwithsysroot/include/SDL -iwithsysroot/include/compat -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-cxx-exceptions -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr -o /tmp/tmpkKeaiw.tmp -x ir /datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/wasm_build/obj/wasm/System.Text.Json.SourceGeneration.Roslyn3.11.Tests.dll.bc [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error : clang -cc1 version 13.0.0 based upon LLVM 13.0.0git default target x86_64-unknown-linux-gnu [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
/datadisks/disk1/work/9D6C08B7/p/build/wasm/WasmApp.Native.targets(293,5): error : emcc: error: '/datadisks/disk1/work/9D6C08B7/p/build/emsdk/upstream/bin/clang++ -target wasm32-unknown-emscripten -DEMSCRIPTEN -fno-inline-functions -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-cxx-exceptions -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr -D__EMSCRIPTEN_major__=2 -D__EMSCRIPTEN_minor__=0 -D__EMSCRIPTEN_tiny__=23 -D_LIBCPP_ABI_VERSION=2 -Dunix -D__unix -D__unix__ -Werror=implicit-function-declaration -Xclang -iwithsysroot/include/SDL --sysroot=/datadisks/disk1/work/9D6C08B7/p/build/emsdk/upstream/emscripten/cache/sysroot -Xclang -iwithsysroot/include/compat -Oz -g -v -c /datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/wasm_build/obj/wasm/System.Text.Json.SourceGeneration.Roslyn3.11.Tests.dll.bc -o /tmp/tmpkKeaiw.tmp' failed (received SIGKILL (-9)) [took 433.767s] [/datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/publish/ProxyProjectForAOTOnHelix.proj]
XHarness artifacts: /datadisks/disk1/work/9D6C08B7/w/AA0A0963/uploads/xharness-output

radical · 2021-09-16T19:12:33Z

Failed to compile /datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/wasm_build/obj/wasm/System.Text.Json.SourceGeneration.Roslyn3.11.Tests.dll.bc -> /datadisks/disk1/work/9D6C08B7/w/AA0A0963/e/wasm_build/obj/wasm/System.Text.Json.SourceGeneration.Roslyn3.11.Tests.dll.o


... received SIGKILL (-9)

@lewing @radekdoulik Could this be due to running out of memory, or other resources?

layomia · 2021-09-16T19:42:07Z

Was chatting with @eerhardt about the AOT failure offline.. FWIW I wanted to note that the size of the test .dll in question here (System.Text.Json.SourceGeneration.Roslyn4.0.Tests) increased from 4.2 MB to 5.4 MB (+ ~28%) with these new changes. I expect the size to keep increasing as we continue sharing tests between src-gen/reflection JsonSerializer implementations.

lewing · 2021-09-16T20:34:10Z

cc @vargaz

layomia · 2021-09-16T21:37:31Z

In a6c3d7c I disabled the src-gen tests in the WASM/AOT leg to unblock this PR so we can get the changes into 6.0. I opened #59228 to track the disabled tests.

layomia · 2021-09-17T04:19:26Z

/backport to release/6.0-rc2

github-actions · 2021-09-17T04:19:39Z

Started backporting to release/6.0-rc2: https://github.com/dotnet/runtime/actions/runs/1244163258

MichalStrehovsky · 2021-09-28T03:04:39Z

I thought RC1 is our go-live release and people can start shipping on it, including e.g. publishing NuGet packages with pregenerated code in it.

Will the NuGet packages using the RC1 S.T.Json source generator be usable on the RTM runtime?

I'm looking at a MissingMethodException: Missing method 'System.Text.Json.Serialization.Metadata.JsonTypeInfo`1<!!0[]> System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateArrayInfo(System.Text.Json.JsonSerializerOptions, System.Text.Json.Serialization.Metadata.JsonTypeInfo, System.Text.Json.Serialization.JsonNumberHandling, System.Action`2<System.Text.Json.Utf8JsonWriter, !!0[]>)' that appears to be caused by using an RC1 source generator to generate the code, but running on a post-RC1 runtime.

danmoseley · 2021-09-28T14:55:58Z

@MichalStrehovsky support for go-live releases like RC1 ends as soon as the next one goes out, RC2 in this case.
https://dotnet.microsoft.com/platform/support/policy/dotnet-core

jkotas · 2021-09-28T15:43:59Z

In the past, we have sometimes treated APIs in the go-live releases as frozen. We are clearly not doing that this time.

I have added dotnet/core#6570 (comment) to make sure we mention this in the .NET 6 RC2 announcement.

layomia added the area-System.Text.Json label Sep 13, 2021

layomia added this to the 6.0.0 milestone Sep 13, 2021

layomia requested review from eiriktsarpalis and steveharter September 13, 2021 17:02

layomia self-assigned this Sep 13, 2021

dotnet-issue-labeler bot added the new-api-needs-documentation label Sep 13, 2021

steveharter reviewed Sep 13, 2021

View reviewed changes

src/libraries/System.Text.Json/ref/System.Text.Json.cs Show resolved Hide resolved

steveharter reviewed Sep 13, 2021

View reviewed changes

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs Outdated Show resolved Hide resolved

danmoseley reviewed Sep 13, 2021

View reviewed changes

src/libraries/System.Text.Json/ref/System.Text.Json.cs Outdated Show resolved Hide resolved

steveharter reviewed Sep 14, 2021

View reviewed changes

...em.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/NodeInteropTests.cs Show resolved Hide resolved

steveharter reviewed Sep 14, 2021

View reviewed changes

...braries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs Show resolved Hide resolved

steveharter approved these changes Sep 14, 2021

View reviewed changes

runfoapp bot mentioned this pull request Sep 14, 2021

TLS1.3 networking tests are flaky on Windows 11 #58927

Closed

layomia force-pushed the Apis branch from 4ce31e9 to a1ac279 Compare September 14, 2021 15:44

layomia added 3 commits September 15, 2021 15:50

Update source-gen APIs according to review

32f39f1

Rename fast-path func name and add src-gen/JsonNode interop support

77efb55

Address review feedback

71b9268

layomia force-pushed the Apis branch from 2bfa8db to 71b9268 Compare September 15, 2021 22:51

layomia mentioned this pull request Sep 16, 2021

System.Text.Json.SourceGeneration.Tests are disabled in WASM #59228

Closed

Disable src-gen tests in browser target OS

a6c3d7c

layomia merged commit 3409f73 into dotnet:main Sep 17, 2021

github-actions bot mentioned this pull request Sep 17, 2021

[release/6.0-rc2] Update source-gen APIs according to review #59243

Merged

sebastienros mentioned this pull request Sep 21, 2021

Platform benchmarks failing for .NET6.0 (System.TypeInitializationException) aspnet/Benchmarks#1699

Closed

MichalStrehovsky mentioned this pull request Sep 28, 2021

System.Text.Json using source generators produces "Method not found" dotnet/runtimelab#1498

Closed

karelz mentioned this pull request Sep 29, 2021

What's new in .NET 6 RC2 [WIP] dotnet/core#6570

Closed

eerhardt mentioned this pull request Oct 5, 2021

Update Regex source generator to use new GetBestTypeByMetadataName method #59778

Merged

ghost locked as resolved and limited conversation to collaborators Nov 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update source-gen APIs according to review #59042

Update source-gen APIs according to review #59042

layomia commented Sep 13, 2021

dotnet-issue-labeler bot commented Sep 13, 2021

ghost commented Sep 13, 2021

layomia commented Sep 13, 2021

steveharter left a comment

steveharter commented Sep 14, 2021

layomia commented Sep 14, 2021

eerhardt commented Sep 16, 2021

radical commented Sep 16, 2021

layomia commented Sep 16, 2021

lewing commented Sep 16, 2021

layomia commented Sep 16, 2021

layomia commented Sep 17, 2021

github-actions bot commented Sep 17, 2021

MichalStrehovsky commented Sep 28, 2021

danmoseley commented Sep 28, 2021

jkotas commented Sep 28, 2021

Update source-gen APIs according to review #59042

Update source-gen APIs according to review #59042

Conversation

layomia commented Sep 13, 2021

dotnet-issue-labeler bot commented Sep 13, 2021

ghost commented Sep 13, 2021

layomia commented Sep 13, 2021

steveharter left a comment

Choose a reason for hiding this comment

steveharter commented Sep 14, 2021

layomia commented Sep 14, 2021

eerhardt commented Sep 16, 2021

radical commented Sep 16, 2021

layomia commented Sep 16, 2021

lewing commented Sep 16, 2021

layomia commented Sep 16, 2021

layomia commented Sep 17, 2021

github-actions bot commented Sep 17, 2021

MichalStrehovsky commented Sep 28, 2021

danmoseley commented Sep 28, 2021

jkotas commented Sep 28, 2021