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

Regression: Semantics of RecordDeclaration has changed #135

Closed
JulianSchuette opened this issue Jul 2, 2020 · 1 comment · Fixed by #205
Closed

Regression: Semantics of RecordDeclaration has changed #135

JulianSchuette opened this issue Jul 2, 2020 · 1 comment · Fixed by #205
Labels
bug Something isn't working
Projects

Comments

@JulianSchuette
Copy link
Contributor

With the revamped type system after version 1.4.2, a few things have changed which break the API/assumptions:

The semantics of RecordDeclaration has changed. Previously, it was "a C++ union/struct/class or Java class that is declared in the analyzed source code". Now it is that, plus "any class/union/struct that is a supertype of the declared classes". This does not really make sense, because for any Record that is not declared in the analyzed scope, the attributes of RecordDeclaration do not make sense: methods, constructors, fields, importStatements etc. cannot be filled.

All clients of CPG will now have to distinguish between different "types" of RecordDeclarations: those that are "real" declarations and can be used as before and those that are only "implicit" Records whose attributes are not available and return null.

Is that change intended? It seems to make life for CPG clients more complicated and dilutes the semantics of RecordDeclaration.

See this example where java.lang.Object is a RecordDeclaration but obviously not declared in the analyzed scope.

recorddecl

@Masrepus
Copy link
Contributor

Masrepus commented Jul 2, 2020

It's actually not just supertypes of classes declared in the analyzed files, but more generally any classes that are used in the analyzed files but whose declarations are somewhere else that is not in scope of the analysed files. So, java.lang.Object is an implicit superclass of every java class, thus used in the code, but not declared in our analysed files. I would think that this behavior is beneficial, e.g. also method calls to methods that we don't have in our scope are routed to dummy methods. This signals that we do know about their existance, as they are needed by some part of the analyzed program, we just don't have their explicit declaration ready. And in this case, the current system doesn't differentiate between unknown methods of known classes and unknown methods of unknown ones.

Masrepus added a commit that referenced this issue Jul 3, 2020
@Masrepus Masrepus added this to the 08/2020 milestone Aug 13, 2020
@Masrepus Masrepus added this to To do in CPG via automation Aug 13, 2020
@Masrepus Masrepus moved this from To do to In progress in CPG Aug 13, 2020
Masrepus added a commit that referenced this issue Aug 14, 2020
Masrepus added a commit that referenced this issue Aug 17, 2020
…known RecordDeclarations, fix tests to adapt to this behavior
@Masrepus Masrepus moved this from In progress to Ready to merge in CPG Aug 20, 2020
CPG automation moved this from Ready to merge to Done Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
CPG
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants