Skip to content

[FIX] Correct get_declaration_of not to skip first symbol #102

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

Conversation

filipsajdak
Copy link
Contributor

While looking for the symbol there is a loop that searches backward using normal iterators. The loop will never check symbols.cbegin() as it is excluded by i != symbols.cbegin() condition.

That makes in the below code we will never find the fun symbol

fun: (inout i : int) -> * const int = {
    return i&;
}

main: () -> int = {
    i := 42;
    p := fun(i);
    // ...
}

This change replaces normal iterators with std::reverse_iterator.

@filipsajdak filipsajdak force-pushed the fsajdak-fix-get_declaration_of-not-to-skip-first-symbol branch 2 times, most recently from 99fd644 to 91163e0 Compare November 11, 2022 22:46
@filipsajdak
Copy link
Contributor Author

I made small mistake that cause skipping one symbol while using std::reverse_iterator fixed in last push.

@filipsajdak filipsajdak force-pushed the fsajdak-fix-get_declaration_of-not-to-skip-first-symbol branch from 91163e0 to 27a13a8 Compare November 28, 2022 23:54
When lookig backward there for loop will never check symbols.cbegin()
as it is excluded by `i != symbols.cbegin()` condition.

Change normal iterators to `std::reverse_iterator`.
@filipsajdak filipsajdak force-pushed the fsajdak-fix-get_declaration_of-not-to-skip-first-symbol branch from 27a13a8 to 7e41f20 Compare November 29, 2022 21:49
@hsutter hsutter closed this in 5355204 Dec 2, 2022
@hsutter
Copy link
Owner

hsutter commented Dec 2, 2022

Thanks! I applied the basic change as a separate commit to also rename the function.

@filipsajdak
Copy link
Contributor Author

Verified on my side on my tests - works!

@filipsajdak
Copy link
Contributor Author

I wondered why I had added a bool value. I just realized that I needed to look outside of the named function in another PR: #96 (I needed that to deduce the type of the symbol - it was needed to deduce if we deal with a pointer or not).

I will rebase other PRs and will see what need to be adjusted

@filipsajdak filipsajdak deleted the fsajdak-fix-get_declaration_of-not-to-skip-first-symbol branch December 22, 2022 10:43
Azmah-Bad pushed a commit to Azmah-Bad/cppfront that referenced this pull request Feb 24, 2023
Renamed `get_declaration_of` to `get_local_declaration_of` to emphasize that this one is intra-function (we'll eventually want a non-local version and that future one should have the non-`local` name)
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