Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multi-namespace class inheritance transpile bug #990

Conversation

luis-soares-sky
Copy link
Contributor

@luis-soares-sky luis-soares-sky commented Dec 20, 2023

Fixes a bug with class inheritance where an ancestor 2+ levels higher would get the wrong super index, but only when that ancestor was a namespace-relative reference from a different namespace than the originating class.

Consider this code example

namespace alpha
    class One
    end class
end namespace

namespace beta
    class Two extends alpha.One
    end class

    class Three extends Two
    end class
end namespace

namespace charlie
    class Four extends beta.Three
    end class
end namespace

The inheritance chain for Four is: Four -> beta.Three -> Two -> alpha.One.
The bug occurs when trying to find Two because it's a namespace-relative lookup. BrighterScript was trying to find charlie.Two instead of beta.Two.

This resulted in the wrong super index, causing a stackoverflow at runtime.

@luis-soares-sky luis-soares-sky changed the title Fix StackOverflow when constructing namespaced grandchild class Fix stack overflow when constructing namespaced grandchild class Dec 20, 2023
@luis-soares-sky luis-soares-sky marked this pull request as ready for review December 20, 2023 15:45
@TwitchBronBron TwitchBronBron changed the title Fix stack overflow when constructing namespaced grandchild class Fix multi-namespace class inheritance transpile bug Dec 20, 2023
@TwitchBronBron TwitchBronBron merged commit c5695e2 into rokucommunity:master Dec 20, 2023
6 checks passed
@luis-soares-sky luis-soares-sky deleted the fix/namespaced-class-super-crash branch December 20, 2023 15:56
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.

2 participants