Skip to content

C++: Do not generate this parameters and read/write side effects from static member functions #3231

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

Merged
merged 3 commits into from
Apr 9, 2020

Conversation

MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented Apr 8, 2020

Previously, when generating IR for code such as

struct A {
  static void f();
};
...
a.f();

(i.e., a qualified call to a static member function as if it was non-static)
we would generate a this parameter and read/write side effect instructions for parameter -1.

This PR removes the 'this' parameter and read/write side effects for such calls. The case for calls of the form A::f() remains unchanged as this was already generated correctly.

This is the simplest fix I could find that still generate the qualifier without using it. That means that we still generate the call to foo() in an example as:

struct A {
  static void f();
};
A* getAnA();
...
getAnA()->f();

even though the result of getAnA() isn't used as a this parameter.

An implication of this implementation is that TranslatedFunctionCall::getQualifier() can exist even though TranslatedFunctionCall::hasQualifier() doesn't match and TranslatedFunctionCall::getQualifierResult doesn't exist.

…is' pointers and side effects for 'this' when accessed through qualifiers
…mber function calls through qualifiers, and accept tests
@MathiasVP MathiasVP added the C++ label Apr 8, 2020
@MathiasVP MathiasVP requested a review from a team as a code owner April 8, 2020 11:42
Copy link
Contributor

@jbj jbj left a comment

Choose a reason for hiding this comment

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

LGTM, but I would welcome a second opinion from @rdmarsh2 or @dbartol.

}

override predicate hasQualifier() {
not exists(MemberFunction func | expr.getTarget() = func and func.isStatic())
Copy link

Choose a reason for hiding this comment

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

Doesn't this hold for all non-member functions as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I've fixed this in 6c7e1cd. There are no changes in the tests as TranslatedFunctionCall::getQualifierResult() would fail for non-member functions anyway.

@dbartol dbartol merged commit 9f18a15 into github:master Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants