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

calls to variables within a type no longer get added to call graph #520

Merged

Conversation

JosephWood2001
Copy link
Contributor

With the addition of adding type calls to the call graph, a new problem arose from the caller of the type call sometimes not being able to find the procedure of the call due to it not existing in the context of the caller, but rather in the context of the type. Similarly, 'calls' that are just arrays aren't getting removed from the call graph because the variable doesn't exist in the context of the caller, but rather of the type, so it isn't detected as a variable.

To fix this, the chain of calls that leads to a call is now packaged with the information of the call. Then, when the calling procedure tries to find the procedure of the call, it will first traverse through the call chain to find the context/type that the call is made on, then from there, it can be checked if the call is not a call, but a variable of the type.

Copy link
Member

@ZedThree ZedThree left a comment

Choose a reason for hiding this comment

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

Thanks again @JosephWood2001 for another PR, it is genuinely appreciated!

This looks good, I just have a few suggestions to tidy up the code. sourceform is probably the most complex part of ford, so it's worth pulling out parts into their own standalone functions. This way, the main code becomes easier to read and understand, and we also get to test those individual parts more thoroughly and easily than having them all inline.

If you could also add a few tests to demonstrate the functionality you've added, that would also be very much appreciated!

ford/sourceform.py Outdated Show resolved Hide resolved
ford/sourceform.py Outdated Show resolved Hide resolved
ford/sourceform.py Outdated Show resolved Hide resolved
ford/sourceform.py Outdated Show resolved Hide resolved
ford/sourceform.py Outdated Show resolved Hide resolved
ford/sourceform.py Outdated Show resolved Hide resolved
ford/sourceform.py Outdated Show resolved Hide resolved
ford/sourceform.py Outdated Show resolved Hide resolved
ford/sourceform.py Outdated Show resolved Hide resolved
Copy link
Member

@ZedThree ZedThree left a comment

Choose a reason for hiding this comment

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

Thanks again @JosephWood2001 for another PR, it is genuinely appreciated!

This looks good, I just have a few suggestions to tidy up the code. sourceform is probably the most complex part of ford, so it's worth pulling out parts into their own standalone functions. This way, the main code becomes easier to read and understand, and we also get to test those individual parts more thoroughly and easily than having them all inline.

If you could also add a few tests to demonstrate the functionality you've added, that would also be very much appreciated!

@JosephWood2001
Copy link
Contributor Author

👍
Ah, I see there are some bracket matching functions in utils, unfortunately I couldn't get either of them to work for my purposes, so I created a new util function adjacent to them. The parsing is now pulled out into a function, and utilizing the new util function, the logic on how it gets to the answer is completely changed (and should be more readable)

The code to traverse the call chain has also been pulled out into it's own function. I also restructured it to no longer use the try except structure that it did in the last commit because I think there was way to much nesting and had a confusing execution flow.

All of your suggestions are appreciated and have been applied.

Copy link
Member

@ZedThree ZedThree left a comment

Choose a reason for hiding this comment

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

Awesome, thanks again @JosephWood2001 !

@ZedThree ZedThree merged commit ff2ac17 into Fortran-FOSS-Programmers:master May 25, 2023
@JosephWood2001 JosephWood2001 deleted the remove-vars-from-calls branch May 25, 2023 14:02
@ZedThree ZedThree added this to the v7.0.0 milestone Aug 16, 2023
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