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

test(AstCheckerTest): Refactor isSurcharged #4119

Merged
merged 4 commits into from
Sep 20, 2021

Conversation

slarse
Copy link
Collaborator

@slarse slarse commented Aug 23, 2021

#4100

This is a refactor of AstCheckerTest#isSurcharged(). It should be noted that I don't understand what "surcharged" means in the context, and I also don't understand the intent of the function. The mechanics are however clear to me, see the invariants in 32de37d.

An invocation v is a potential surcharge delegate if it:

  • Is an invocation that is the only statement in the body.
  • Is an invocation that is the first statement in the body, and the last statement is a return without an invocation.
  • Is an invocation in a return occurring as the last statement in the body.

v is determined to be a surcharge delegate if it fulfills any of the 3 above points, and returns false for isToBeProcessed(v).

@slarse slarse changed the title wip: refactor(AstCheckerTest): Refactor isSurcharged review: refactor(AstCheckerTest): Refactor isSurcharged Sep 18, 2021
@slarse
Copy link
Collaborator Author

slarse commented Sep 18, 2021

I was going to keep going here but I think this refactoring will be sufficient.

Ping @monperrus for review, I need to get on with the rest of this class so that I unblock @I-Al-Istannen's PR #4038 which has been blocked for way too long.

@monperrus monperrus changed the title review: refactor(AstCheckerTest): Refactor isSurcharged test(AstCheckerTest): Refactor isSurcharged Sep 20, 2021
@monperrus monperrus merged commit 656254e into INRIA:master Sep 20, 2021
@monperrus
Copy link
Collaborator

ack, thanks for the test improvement!

@slarse slarse deleted the issue/4100-refactor-is-surcharged branch September 20, 2021 11:24
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