DapperLib / DapperAOT

Build time tools in the flavor of Dapper
Other
349 stars 19 forks source link

Stack overflow in source generator? #94

Closed DamianEdwards closed 7 months ago

DamianEdwards commented 8 months ago

I tried to setup one of the ASP.NET Core benchmarks app's (this one) to use Dapper.AOT but it appears to be causing a stack overflow with lots of the following printed to output during build:

C:\Program Files\dotnet\sdk\8.0.100\Roslyn\Microsoft.CSharp.Core.targets(84,5): error :    at Dapper.Internal.Inspection.InvolvesTup
leType(Microsoft.CodeAnalysis.ITypeSymbol, Boolean ByRef) [D:\src\GitHub\aspnet\Benchmarks\src\BenchmarksApps\TodosApi\TodosApi.cspr
oj]

Visual Studio freezes and then disappears.

To Reproduce Project that I was updating is: https://github.com/aspnet/Benchmarks/tree/damianedwards/dapper.aot/src/BenchmarksApps/TodosApi

Expected behavior Expect it to work.

Screenshots See error above.

Additional context none

mgravell commented 8 months ago

A quick attempt to repro was unsuccessful; I suspect that this is a nuanced thing that involves your migration and wrapper code from DbDataSource (which isn't directly supported in Dapper until vNext). Any chance you have an intermediate branch with your changes that show this behaviour?

DamianEdwards commented 8 months ago

The link is to the intermediate branch with these changes 😁 Let me try to repro on a different machine.

DamianEdwards commented 8 months ago

I just repro'ed this on another machine using the branch at https://github.com/aspnet/Benchmarks/tree/damianedwards/dapper.aot/

Clone that, then navigate to ./src/BenchmarksApps/TodosApi and run dotnet build.

mgravell commented 7 months ago

OK, this was a PITA to investigate - it seems to be fixed in 1.0.27, but getting a proper reliable repro was elusive, even using a local nuget folder and proper restore; however, it is either something to do with error-types or unbound generics; I've fixed both of those scenarios; I've also added a general depth check, and better fault storage

yikes, that was ... fun

for small values of "fun"