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

Make dummy creation for calls consistent #205

Merged
merged 7 commits into from Aug 27, 2020

Conversation

Masrepus
Copy link
Contributor

@Masrepus Masrepus commented Aug 20, 2020

Previously, calls to unknown methods led to creation of dummy methods/functions in some situations, but not in all. This closes #135, as we now

  • Don't create dummies for methods that would be located in classes whose record declaration is not in our graph (e.g. System.out.println())
  • Do create dummies for any call to an unknown method that can be presumed to be located in known classes or translation units.

Of course, we can't be 100% sure when analyzing this statically, e.g. as there could be methods that are declared in header files that we don't know. In this case, the call looks like a call to a local function or method of the current class, depending on its location. But in my opinion this is just something one has to live with when wanting to analyze even incomplete code to some degree.

Graph Changes

Calls to unknown functions/methods leads to creation of empty function/method declarations whose name, return type and signature matches the corresponding call expression. No new record declarations or translation units are created.

@Masrepus Masrepus added the graph-changing This change breaks the current graph structure, i.e. it changes semantic of properties and edges label Aug 20, 2020
@@ -66,4 +62,13 @@ public static ConstructorDeclaration from(MethodDeclaration methodDeclaration) {

return c;
}

@Override
public void setRecordDeclaration(@Nullable RecordDeclaration recordDeclaration) {
Copy link
Member

Choose a reason for hiding this comment

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

Is that method actually called? Somehow does not look like it? Shouldn't it be called where you cut the original code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is called, e.g. in NodeBuilder.newConstructorDeclaration. What do you mean by "where you cut the original code"?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, i didn't see that it was overriding another function. I was refereeing to the lines 50-54 in the original code where you cut the code from and pasted in this new function. Looks ok then

@oxisto
Copy link
Member

oxisto commented Aug 25, 2020

I will ask @fwendland to do a quick check if this breaks anything in codyze, then we can merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graph-changing This change breaks the current graph structure, i.e. it changes semantic of properties and edges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: Semantics of RecordDeclaration has changed
3 participants