DebuggableAttribute frequent "could not decode" problem

[tamlin-mike]

[assembly: Debuggable(/*Could not decode attribute arguments.*/)]

seems to be quite frequent recently, for me. Sometimes (!) it displays properly, then... not.

IL example from a problematic assembly

.custom instance void [netstandard]System.Diagnostics.DebuggableAttribute::.ctor(valuetype [netstandard]System.Diagnostics.DebuggableAttribute/DebuggingModes) = (
	01 00 07 01 00 00 00 00
)

To save time (even that the followin probably deserves an own issue), this also can provoke a crash, when trying to display a popup tooltip for that error comment:

---------------------------
Sorry, we crashed
---------------------------
System.NullReferenceException: Object reference not set to an instance of an object.

   at ICSharpCode.ILSpy.TextView.DecompilerTextView.FlowDocumentTooltip..ctor(FlowDocument document) in W:\ILSpy_github\ILSpy\ILSpy\TextView\DecompilerTextView.cs:line 398

   at ICSharpCode.ILSpy.TextView.DecompilerTextView.GenerateTooltip(ReferenceSegment segment) in W:\ILSpy_github\ILSpy\ILSpy\TextView\DecompilerTextView.cs:line 348

   at ICSharpCode.ILSpy.TextView.DecompilerTextView.TextViewMouseHover(Object sender, MouseEventArgs e) in W:\ILSpy_github\ILSpy\ILSpy\TextView\DecompilerTextView.cs:line 195

   at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)

   at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs)

   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)

   at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)

   at ICSharpCode.AvalonEdit.Rendering.MouseHoverLogic.OnMouseHover(MouseEventArgs e)

   at System.Windows.Threading.DispatcherTimer.FireTick(Object unused)

   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)

   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
---------------------------
OK   
---------------------------

[siegfriedpammer]

[assembly: Debuggable(/*Could not decode attribute arguments.*/)]

seems to be quite frequent recently, for me. Sometimes (!) it displays properly, then... not.

This most likely happens because ILSpy does not resolve referenced assemblies when you first click on an assembly node. This is because, we want ILSpy to be more responsive. The enum value cannot be decoded, because the reference to the type [netstandard]System.Diagnostics.DebuggableAttribute/DebuggingModes will not be resolved.

[tamlin-mike]

Gotcha. So long as it's fully resolved when saving as project, I can live with that.

Though... would it become too hairy to kick off resolving in another thread? On the other hand, that would also become confusing, if the view all of a sudden started changing while you watched in, in time with types getting resolved. Nevermind, just thinking out loud.

[dgrunwald]

Viewing any class/method will automatically load additional assemblies; but we suppress this for the assembly node itself. After all, the user might be selecting an assembly in order to remove it from the list, in which case it would be highly inappropriate to add its dependencies to the assembly list.

I guess we could display a "Load dependencies" button next to the /*Could not decode attribute arguments.*/ comment.

[dgrunwald]

Or if it's really just DebuggableAttribute where this happens so frequently, maybe just hardcode some knowledge about the DebuggingModes enum into the decompiler.

[tamlin-mike]

but we suppress this for the assembly node itself

Sound reasoning.

In light of that, your idea is not entirely unreasonable, to also do some special-handling, limited to this special case - when dependency loading is intentionally prevented. It would be of very limited scope, while helping display potentially valuable information that was hidden behind a comment by another special-case. If nothing else, it would stop making is seem like ILSpy doesn't even have the ability to decode this.

While DebuggableAttribute is the most frequent, now checking I also came across SecurityRules. So maybe, to generalise and not put too much effort in it, simply display the int value (optionally with a comment somewhere, to the effect of "symbolic constants lookup disabled while viewing only top node") for enums when in this state? It would at least provide a minimum amount of information instead of a (slightly misleading) comment. Should it become a large enough annoyance, it would then be easy to go back and flesh it out with symbolic names on a case-by-case basis.

I'm not sure it's a large enough annoyance to warrant the work to place a "Load dependencies" button, not to mentions in some esoteric case that could potentially become a lot of buttons. :-)

[dgrunwald]

We can't display the integer value either: without the enum definition, we don't know the size of the underlying integer type, so we don't know how many bytes to read from the blob. Though I think we know the overall size of the attribute blob, so I guess we could theoretically special-case the last argument, detecting the size of the underlying integer from the remaining bytes in the blob. But that would require writing our own attribute decoder instead of using the one from System.Reflection.Metadata.

[tamlin-mike]

This is starting to look more hairy and like a lot more work than I would have hoped it would be.

Changing the comment to better reflect the state, instead of suggesting ILSpy is unable to decode it, would in my mind be enough. I don't find it a large enough annoyance to warrant so much extra work for something so, in reality, insignificant.

[siegfriedpammer]

Would it be a good enough solution to include the reason for the decoding failure? For example: /*Could not decode attribute arguments: missing type reference */

What do you think?

[tamlin-mike]

I'm not sure missing is the right word. It could imply the type is nowhere to be found. On the other hand, something like "type not loaded" could also look strange to a casual observer; "Why isn't it loaded? It seems to be needed here to display proper information".

Would "... arguments: type loading deferred" work? That both explains the ILSpy internal condition for the "failure" and more importantly why it's not displaying properly.

[IanKemp]

[assembly: Debuggable(/*Could not decode attribute arguments.*/)] seems to be quite frequent recently, for me. Sometimes (!) it displays properly, then... not.

This most likely happens because ILSpy does not resolve referenced assemblies when you first click on an assembly node. This is because, we want ILSpy to be more responsive. The enum value cannot be decoded, because the reference to the type [netstandard]System.Diagnostics.DebuggableAttribute/DebuggingModes will not be resolved.

I don't agree with the responsiveness argument. This is a very specific tool for a very specific purpose, and one of those purposes is viewing information about assemblies. Forcing users to manually load additional assemblies to get at that information is IMO a far worse experience than a couple of seconds hang when loading one.

At the very least, an application option to either load these on-demand, or manually load them (as currently) would be useful.

I'm not sure missing is the right word. It could imply the type is nowhere to be found. On the other hand, something like "type not loaded" could also look strange to a casual observer; "Why isn't it loaded? It seems to be needed here to display proper information".

Would "... arguments: type loading deferred" work? That both explains the ILSpy internal condition for the "failure" and more importantly why it's not displaying properly.

A combination of the above and https://github.com/icsharpcode/ILSpy/issues/1776#issuecomment-549158594 would IMO be the best solution.

My suggestion: a "Load dependencies" button between after the comments about the assembly at the top of the view, and the using statements. Then each attribute states /* Dependent type not loaded, click "Load dependencies" button above to view */.