Avoid an infinite cycle in MakeAcyclicBaseType
#81005
Conversation
| while ((object)current != null); | ||
|
|
||
| diagnostics.Add(useSiteInfo.Diagnostics.IsNullOrEmpty() ? Location.None : (FindBaseRefSyntax(declaredBase) ?? GetFirstLocation()), useSiteInfo); | ||
| diagnostics.Add(useSiteInfo.Diagnostics.IsNullOrEmpty() ? Location.None : ((reportAtFirstLocation ? null : FindBaseRefSyntax(declaredBase)) ?? GetFirstLocation()), useSiteInfo); |
There was a problem hiding this comment.
How exactly did FindBaseRefSyntax cause a cycle? If it cannot handle special types for some reason, shouldn't that be fixed instead to avoid similar issues in the future? #Resolved
There was a problem hiding this comment.
How exactly did
FindBaseRefSyntaxcause a cycle? If it cannot handle special types for some reason, shouldn't that be fixed instead to avoid similar issues in the future?
Enum is the only declaration that syntactically has a base list that never contributes to the declared base value. FindBaseRefSyntax looks for a type reference in a base list. It attempts to bind types in the list, lookup needs to know base type, and for an enum, we end up here again, due to the same reason - base type is not in the base list, when we are binding types in it, we are not resolving bases. For scenarios when declaredBase isn't a result of binding of a name from base list, it doesn't make sense trying to find the type in that list.
There was a problem hiding this comment.
I wonder whether we should just have FindBaseRefSyntax bail early for structs and enums instead. That would also have the benefit of preventing such a cycle from sneaking back in in the future if we need to get the base syntax in a new location.
There was a problem hiding this comment.
I wonder whether we should just have
FindBaseRefSyntaxbail early forstructs andenums instead. That would also have the benefit of preventing such a cycle from sneaking back in in the future if we need to get the base syntax in a new location.
I don't think this is the right thing to do. The purpose of FindBaseRefSyntax is to find type reference in the base list. There is nothing wrong in using it for enum type in general. It is not appropriate for the method to decide whether it is Ok to look into the list. This specific use of the method is troublesome, especially because, in this particular case, we are looking for something that we know isn't there.
|
@dotnet/roslyn-compiler For a second review |
3 participants
Fixes #80987