Skip to content

Commit

Permalink
Merge branch 'main' into codeql/upgrade-to-2.14.6
Browse files Browse the repository at this point in the history
  • Loading branch information
lcartey authored Oct 24, 2023
2 parents 91fd477 + c719039 commit 18eae16
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 21 deletions.
7 changes: 7 additions & 0 deletions change_notes/2023-09-28-a7-3-1-updates.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
* `A7-3-1` - `HiddenInheritedNonOverridableMemberFunction.ql`:
- Reduce duplication by reporting only a single location for each declaration of a problematic element.
- Reduce duplication when reporting the hidden function by reporting only one declaration entry.
- Improve performance by eliminating a number of bad join orders.
- Fix false positives where the using declaration occurred after the function declaration.
- Exclude special member functions, which cannot be inherited.
- Exclude private member functions, which cannot be inherited.
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,45 @@

import cpp
import codingstandards.cpp.autosar
import codingstandards.cpp.Class

from FunctionDeclarationEntry overridingDecl, FunctionDeclarationEntry hiddenDecl
/**
* Holds if the class has a non-virtual member function with the given name.
*/
pragma[noinline, nomagic]
predicate hasNonVirtualMemberFunction(Class clazz, MemberFunction mf, string name) {
mf.getDeclaringType() = clazz and
mf.getName() = name and
not mf.isVirtual() and
// Exclude private member functions, which cannot be inherited.
not mf.isPrivate()
}

/**
* Holds if the member function is in a class with the given base class, and has the given name.
*/
pragma[noinline, nomagic]
predicate hasDeclarationBaseClass(MemberFunction mf, Class baseClass, string functionName) {
baseClass = mf.getDeclaringType().getABaseClass() and
functionName = mf.getName()
}

from MemberFunction overridingDecl, MemberFunction hiddenDecl, Class baseClass, string name
where
not isExcluded(overridingDecl, ScopePackage::hiddenInheritedNonOverridableMemberFunctionQuery()) and
// Check if we are overriding a non-virtual inherited member function
overridingDecl.getName() = hiddenDecl.getName() and
overridingDecl.getDeclaration().getDeclaringType().getABaseClass() =
hiddenDecl.getDeclaration().getDeclaringType() and
not hiddenDecl.getDeclaration().isVirtual() and
hasNonVirtualMemberFunction(baseClass, hiddenDecl, name) and
hasDeclarationBaseClass(overridingDecl, baseClass, name) and
// Where the hidden member function isn't explicitly brought in scope through a using declaration.
not exists(UsingDeclarationEntry ude |
ude.getDeclaration() = hiddenDecl.getDeclaration() and
ude.getEnclosingElement() = overridingDecl.getDeclaration().getDeclaringType() and
ude.getLocation().getStartLine() < overridingDecl.getLocation().getStartLine()
ude.getDeclaration() = hiddenDecl and
ude.getEnclosingElement() = overridingDecl.getDeclaringType()
) and
// Exclude compiler generated member functions which include things like copy constructor that hide base class
// copy constructors.
not overridingDecl.getDeclaration().isCompilerGenerated()
not overridingDecl.isCompilerGenerated() and
// Exclude special member functions, which cannot be inherited.
not overridingDecl instanceof SpecialMemberFunction
select overridingDecl,
"Declaration for member '" + overridingDecl.getName() +
"' hides non-overridable inherited member function $@", hiddenDecl, hiddenDecl.getName()
"Declaration for member '" + name + "' hides non-overridable inherited member function $@",
hiddenDecl, hiddenDecl.getName()
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ where
not isExcluded(overridingDecl, ScopePackage::hiddenInheritedOverridableMemberFunctionQuery()) and
// Check if we are overriding a virtual inherited member function
hiddenDecl.getDeclaration().isVirtual() and
// Exclude private member functions, which cannot be inherited.
not hiddenDecl.getDeclaration().(MemberFunction).isPrivate() and
// The overriding declaration hides the hidden declaration if:
(
// 1. the overriding declaration overrides a function in a base class that is an overload of the hidden declaration
Expand All @@ -36,8 +38,7 @@ where
// and the hidden declaration isn't explicitly brought in scope through a using declaration.
not exists(UsingDeclarationEntry ude |
ude.getDeclaration() = hiddenDecl.getDeclaration() and
ude.getEnclosingElement() = overridingDecl.getDeclaration().getDeclaringType() and
ude.getLocation().getStartLine() < overridingDecl.getLocation().getStartLine()
ude.getEnclosingElement() = overridingDecl.getDeclaration().getDeclaringType()
)
or
// 2. if the overriding declaration doesn't override a base member function but has the same name
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
| test.cpp:42:6:42:7 | declaration of f1 | Definition for 'f1' is not available for unqualified lookup because it is declared after $@ | test.cpp:39:12:39:13 | using f1 | using-declaration |
| test.cpp:46:6:46:7 | declaration of f1 | Definition for 'f1' is not available for unqualified lookup because it is declared after $@ | test.cpp:43:12:43:13 | using f1 | using-declaration |
Original file line number Diff line number Diff line change
@@ -1 +1 @@
| test.cpp:16:8:16:9 | declaration of f1 | Declaration for member 'f1' hides non-overridable inherited member function $@ | test.cpp:7:8:7:9 | declaration of f1 | f1 |
| test.cpp:20:8:20:9 | f1 | Declaration for member 'f1' hides non-overridable inherited member function $@ | test.cpp:7:8:7:9 | f1 | f1 |
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
| test.cpp:18:8:18:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:9:16:9:17 | declaration of f2 | f2 |
| test.cpp:18:8:18:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:11:16:11:17 | declaration of f2 | f2 |
| test.cpp:23:8:23:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:9:16:9:17 | declaration of f2 | f2 |
| test.cpp:23:8:23:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:10:16:10:17 | declaration of f2 | f2 |
| test.cpp:23:8:23:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:11:16:11:17 | declaration of f2 | f2 |
| test.cpp:22:8:22:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:9:16:9:17 | declaration of f2 | f2 |
| test.cpp:22:8:22:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:11:16:11:17 | declaration of f2 | f2 |
| test.cpp:27:8:27:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:9:16:9:17 | declaration of f2 | f2 |
| test.cpp:27:8:27:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:10:16:10:17 | declaration of f2 | f2 |
| test.cpp:27:8:27:9 | declaration of f2 | Declaration for member 'f2' hides overridable inherited member function $@ | test.cpp:11:16:11:17 | declaration of f2 | f2 |
34 changes: 33 additions & 1 deletion cpp/autosar/test/rules/A7-3-1/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ class C1 {
virtual void f2(int);
virtual void f2(double);
virtual void f2(S1);

private:
void f3(int);
void f4(int);
};

class C2 : public C1 {
Expand Down Expand Up @@ -47,7 +51,7 @@ void f1() {
l1.f1(0); // calls C2::f1(double) instead of C1::f1(int)
l1.f2(0); // calls C2::f2(double) instead of C1::f2(int)
// S1 s1;
// l1.f2(s1); Won't compile because there is no suitable conversion fro S1 to
// l1.f2(s1); Won't compile because there is no suitable conversion from S1 to
// double.
C1 &l2{l1};
l2.f1(0); // calls C1::f1(int)
Expand All @@ -60,3 +64,31 @@ void f1() {
S1 l4;
l3.f2(l4); // calls C1:f2(S1)
}

class C5 : public C1 {
public:
void f1(double); // COMPLIANT
using C1::f1; // order of using and f1 declaration is not relevant

void f2(double) override; // COMPLIANT
using C1::f2; // order of using and f2 declaration is not relevant
};

void f2() {
C5 c5;
c5.f1(0); // calls C1::f1(int)
c5.f1(0.0); // calls C5::f1(double)
c5.f2(0); // calls C1::f2(int)
c5.f2(0.0); // calls C5::f2(double)
}

class C6 : public C1 {
public:
C6 &operator=(const C6 &); // COMPLIANT
};

class C7 : public C1 {
void f3(int); // COMPLIANT

void f4(int); // COMPLIANT
};

0 comments on commit 18eae16

Please sign in to comment.