Skip to content

Conversation

@calumgrant
Copy link
Contributor

When a base class is unknown (due to a missing reference), Roslyn will incorrectly give a base class of Object, which gets written to the extends/2 table. This can cause DB inconsistencies. Instead, do not populate Object as a base class, and instead deduce it in QL.

@calumgrant calumgrant marked this pull request as ready for review November 10, 2019 22:04
@calumgrant calumgrant requested a review from a team as a code owner November 10, 2019 22:04
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small thing, and also a dist-compare run would be good.

else (
not this instanceof ObjectType and
result instanceof ObjectType
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would write

extend(this, getTypeRef(result))
or
not extend(this, _) and
not this instanceof ObjectType and
result instanceof ObjectType

…correctly give a base class of 'Object' when a base type is missing, which is often incorrect and can lead to db inconsistencies.
@adityasharad adityasharad changed the base branch from master to main August 14, 2020 18:35
@adityasharad adityasharad requested a review from a team as a code owner August 14, 2020 18:35
or
not extend(this, _) and
not this instanceof ObjectType and
result instanceof ObjectType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this result instanceof ObjectType and not result = ObjectType?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter is not valid QL syntax, as the right-hand-side is a QL class and not an expression. result = any(ObjectType ot) would work, but instanceof is better IMO. Note that the extractor will guarantee that there is indeed only one element inside ObjectType.

@hvitved
Copy link
Contributor

hvitved commented Aug 17, 2020

I had to make a similar change on #4017.

@calumgrant calumgrant closed this Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants