Open ArchieCoder opened 9 months ago
Do you have any memory of what code was being typed when this happened or a dump when it happened? Based on the stack alone this likely can't be tracked down. That code is pretty straight forward and only a violation of some fairly core assumptions in our model could produce it. That is likely what is happening but need a code sample to get us pointed in the right direction.
@jaredpar I get ~10-15 exceptions per day.
Here is a screenshot earlier this morning. I was only replacing a word after duplicating a line.
https://github.com/dotnet/roslyn/issues/72074#issuecomment-1941770433
@ArchieCoder
Do you have any of the dumps for those exceptions though? The stack alone here is not going to be enough for us to root cause the problem.
This particular exception is part of conditional access binding ?.
, if you have any cases of that in the code that you're editing that would be a good start.
How do I enable VS to have a dump when this is happening?
I suspect this issue will be heavily mitigated in 17.10 Preview 1 by #68214
@ArchieCoder I'm not sure we have a way to collect a dump directly at the point of failure, but if you have a way to reproduce this in a standalone file it should be easy to identify the cause once we can reproduce it.
Video and 3 stack traces: https://github.com/dotnet/roslyn/assets/1608424/96c7a574-0698-4ae8-a542-281239f0aeb1 https://github.com/dotnet/roslyn/assets/1608424/5965fc36-87e9-48c9-ac37-093431e58910
I can share the source file if you provide me a private way.
If you file a VS feedback bug it will provide a way to privately share files.
The fastest way to move this forward is to grab a dump of the crash when it happens. The videos don't really help here cause without the entire solution it's not providing much more information than a stack trace. I'm skeptical a single source file is going to help here.
@jaredpar Ok, I did the steps you suggested, the dump should be included now: https://developercommunity.visualstudio.com/t/Exceptions-happens-when-duplicating-a-li/10584059
Your colleague above was not aware about the recorder (and I also forgot).
@jaredpar given:
System.NullReferenceException : Object reference not set to an instance of an object.
at Microsoft.CodeAnalysis.CSharp.SyntaxFactory.FindConditionalAccessNodeForBinding(CSharpSyntaxNode node)
at Microsoft.CodeAnalysis.CSharp.Binder.GetReceiverForConditionalBinding(ExpressionSyntax binding,BindingDiagnosticBag diagnostics)
at Microsoft.CodeAnalysis.CSharp.Binder.BindMemberBindingExpression(MemberBindingExpressionSyntax node,Boolean invoked,Boolean indexed,BindingDiagnosticBag diagnostics)
at Microsoft.CodeAnalysis.CSharp.Binder.BindExpressionInternal(ExpressionSyntax node,BindingDiagnosticBag diagnostics,Boolean invoked,Boolean indexed)
and that this is within an IDE rewrite, it's pretty likely we're producing an invalid tree that is breaking you. 'Binding' nodes (?.
, ?[
, ...) in particular are very easy to get wrong in a rewrite. So i would not be surprised if we made some simple assumptions about tree shape that don't hold here.
@ArchieCoder The recorder captures the state, but not at a point in time where the context leading to the exception is available. By the time you see the gold bar appear, the context information is already discarded. A standalone project would be extremely helpful in narrowing this down if there is any way you can create it. Otherwise, we may be able to identify the source of the issue by looking at the source code if it's attached to the issue you opened yesterday.
I was able to do a sample solution.
I reduced the sample solution to the following. Pressing Ctrl+. on the assignment to IsDetailsPanelShown
and selecting "Introduce parameter for 'IsDetailsPanelShown'" will result in an exception.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="CommunityToolkit.Mvvm" Version="8.2.2" />
</ItemGroup>
</Project>
using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Input;
namespace NMaxDashboard.Core.ViewModels;
public abstract partial class BaseDatePageViewModel : ObservableObject
{
[RelayCommand]
public virtual async Task Load()
{
}
}
public partial class PowerDataPageViewModel : BaseDatePageViewModel
{
[ObservableProperty] private bool _isDetailsPanelShown;
public override async Task Load()
{
IsDetailsPanelShown = true;
}
}
I was able to further simplify this to reproduce without any use of source generators. The source of the bug in Introduce Parameter is the method group reference to Load
. This may be a slightly different exception than the one at the start of this issue though.
using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Input;
namespace NMaxDashboard.Core.ViewModels;
public abstract partial class BaseDatePageViewModel : ObservableObject
{
private AsyncRelayCommand? loadCommand;
public IAsyncRelayCommand LoadCommand => loadCommand ??= new AsyncRelayCommand(new Func<Task>(Load));
public virtual async Task Load()
{
}
}
public partial class PowerDataPageViewModel : BaseDatePageViewModel
{
private bool IsDetailsPanelShown;
public override async Task Load()
{
IsDetailsPanelShown = true;
}
}
@CyrusNajmabadi I think the following three changes would work:
@akhera99 for intro-param issues.
Update Rename tracking code fix to not try to rename items defined in generated code
taht seems fine with me.
if such a change would be needed present a warning in the preview that the result might not be correct
The last time this was discussed, we said that we should not give warnings for generated code. Because the presumption is that the generated code will just regen based on the changes actually made. And if we trust generators to be good, they should get the code back to a good state. In otehr words, it doesn't make sense to give warning against a change, when teh part we're warning about is stale code.
Latest VS version. Exception happens while coding.