Ericsson / clang

Cross Translation Unit analysis capability for Clang Static Analyzer. (Fork of official clang at http://llvm.org/git/clang)
http://clang.llvm.org/
Other
15 stars 10 forks source link

Structural equivalence does not check member functions #378

Closed martong closed 5 years ago

martong commented 6 years ago

There are cases when the check should indeed do this, otherwise we falsely report equivalency between a forward decl and a definition. See #377

Task: Try to implement member function check as well and see if tests (including lldb) and analysis on projects does not have regression.

Xazax-hun commented 6 years ago

I do not really see what is the connection between function checks and messing up forward declarations with definitions. I think the latter should be checked separately at call site when these should be considered different. (Unless this should behave the same for all call sites. )

martong commented 6 years ago

Ok, generally I agree, that we should change this only if we are confident that all calls handle this consistently.

I do not really see what is the connection between function checks and messing up forward declarations with definitions.

Let's assume a class template with a fwd decl and a definitiion:

template <class T>
struct B;

template <class T>
struct B {};

The templated CXXRecordDecl for the definition and the templated CXXRecordDecl for the forward decl will be structurally equivalent. Because there are no data members. If we had checked the member functions, then the implicitly generated ctor, dtor would fire the difference. Making the fwd decl equal to the definition confuses the lookup logic, see #377

Xazax-hun commented 6 years ago

I would prefer checking explicitly for being a forward declaration rather than relying on the implicit declarations.

martong commented 6 years ago

Okay, that is what we do exactly in #377.

On the other hand, the precise definition of structural equivalence should imply the same number of member functions with the same types, shouldn't it?

martong commented 6 years ago

We already do the same thing with the friend functions, so it is kind of weird we don't include member functions but we include free functions. We should do neither or both for the sake of consistency.

balazske commented 6 years ago

This "equivalence" should mean that all members (variables, functions) are equivalent (unless this check is used to test something like memory layout).

martong commented 5 years ago

Now we check CXXMethodDecls independently from FunctionDecls. However we do not check the them when we check the equivalency of two CXXRecordDecls.