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

Destructor usage is now properly registered #654

Merged
merged 3 commits into from
Dec 4, 2023

Conversation

dbukki
Copy link
Collaborator

@dbukki dbukki commented Oct 22, 2023

Currently supported places of usage:

  • In delete expressions (for heap-allocated variables).
  • At the end of the parent scope (for stack-allocated local variables).
  • At the end of the destructor of enclosing record types (for member variables).

Fixes #306.

Only one question so far at line 947: is there a better solution than this?

Supported places of usage:
- In delete expressions (for heap-allocated variables).
- At the end of the parent scope (for stack-allocated local variables).
- At the end of the destructor of enclosing record types (for member variables).
@dbukki dbukki self-assigned this Oct 22, 2023
A new _statements stack now stores all parent statements of the currently visited statement.
@mcserep mcserep added Kind: Enhancement 🌟 Plugin: C++ Issues related to the parsing and presentation of C++ projects. labels Nov 1, 2023
@mcserep mcserep added this to the Release Gershwin milestone Nov 1, 2023
@mcserep
Copy link
Collaborator

mcserep commented Nov 1, 2023

Only one question so far at line 947: is there a better solution than this?

@dbukki I am not sure which line you mean, line 947 seems to be unchanged in the GitHub diff viewer. Please mark the respective line with a comment in the PR.

@dbukki
Copy link
Collaborator Author

dbukki commented Nov 2, 2023

@dbukki I am not sure which line you mean, line 947 seems to be unchanged in the GitHub diff viewer. Please mark the respective line with a comment in the PR.

In my first commit, in VisitStmt there was a FIXME comment that we've also discussed during our last weekly meeting. It was about whether there is a better solution to retrieving the parent statement of a statement than using the parent map.

In my second commit on this PR, I solved this by using a similar stack-based approach that we've already had examples for in the code.

plugins/cpp/parser/src/clangastvisitor.h Outdated Show resolved Hide resolved
plugins/cpp/parser/src/clangastvisitor.h Outdated Show resolved Hide resolved
plugins/cpp/parser/src/clangastvisitor.h Outdated Show resolved Hide resolved
plugins/cpp/parser/src/clangastvisitor.h Show resolved Hide resolved
@dbukki dbukki requested a review from whisperity November 11, 2023 11:15
Copy link
Collaborator

@whisperity whisperity left a comment

Choose a reason for hiding this comment

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

Apologies for being late with this and that @mcserep had to ping me extensively. I get like 1000 mails from GitHub every day thanks to llvm/llvm-project.

I think this is good to go with the changes applied.

@mcserep mcserep merged commit 0709f4e into Ericsson:master Dec 4, 2023
7 checks passed
@dbukki dbukki deleted the destructor-usage branch December 9, 2023 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kind: Enhancement 🌟 Plugin: C++ Issues related to the parsing and presentation of C++ projects.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Destructor call is not present on delete expressions.
3 participants