diff --git a/change_notes/2023-09-28-a7-3-1-updates.md b/change_notes/2023-09-28-a7-3-1-updates.md new file mode 100644 index 0000000000..f56d706e74 --- /dev/null +++ b/change_notes/2023-09-28-a7-3-1-updates.md @@ -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. \ No newline at end of file diff --git a/cpp/autosar/src/rules/A7-3-1/HiddenInheritedNonOverridableMemberFunction.ql b/cpp/autosar/src/rules/A7-3-1/HiddenInheritedNonOverridableMemberFunction.ql index 0083fb0cb4..fa1859c229 100644 --- a/cpp/autosar/src/rules/A7-3-1/HiddenInheritedNonOverridableMemberFunction.ql +++ b/cpp/autosar/src/rules/A7-3-1/HiddenInheritedNonOverridableMemberFunction.ql @@ -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() diff --git a/cpp/autosar/src/rules/A7-3-1/HiddenInheritedOverridableMemberFunction.ql b/cpp/autosar/src/rules/A7-3-1/HiddenInheritedOverridableMemberFunction.ql index 4a252ecb51..437c8798f9 100644 --- a/cpp/autosar/src/rules/A7-3-1/HiddenInheritedOverridableMemberFunction.ql +++ b/cpp/autosar/src/rules/A7-3-1/HiddenInheritedOverridableMemberFunction.ql @@ -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 @@ -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 diff --git a/cpp/autosar/test/rules/A7-3-1/DefinitionNotConsideredForUnqualifiedLookup.expected b/cpp/autosar/test/rules/A7-3-1/DefinitionNotConsideredForUnqualifiedLookup.expected index ea0f998533..1a1e0e5297 100644 --- a/cpp/autosar/test/rules/A7-3-1/DefinitionNotConsideredForUnqualifiedLookup.expected +++ b/cpp/autosar/test/rules/A7-3-1/DefinitionNotConsideredForUnqualifiedLookup.expected @@ -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 | diff --git a/cpp/autosar/test/rules/A7-3-1/HiddenInheritedNonOverridableMemberFunction.expected b/cpp/autosar/test/rules/A7-3-1/HiddenInheritedNonOverridableMemberFunction.expected index 85ede1ada2..dc762e5a2d 100644 --- a/cpp/autosar/test/rules/A7-3-1/HiddenInheritedNonOverridableMemberFunction.expected +++ b/cpp/autosar/test/rules/A7-3-1/HiddenInheritedNonOverridableMemberFunction.expected @@ -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 | diff --git a/cpp/autosar/test/rules/A7-3-1/HiddenInheritedOverridableMemberFunction.expected b/cpp/autosar/test/rules/A7-3-1/HiddenInheritedOverridableMemberFunction.expected index c2f8ee1f40..2e0e8809e8 100644 --- a/cpp/autosar/test/rules/A7-3-1/HiddenInheritedOverridableMemberFunction.expected +++ b/cpp/autosar/test/rules/A7-3-1/HiddenInheritedOverridableMemberFunction.expected @@ -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 | diff --git a/cpp/autosar/test/rules/A7-3-1/test.cpp b/cpp/autosar/test/rules/A7-3-1/test.cpp index 7aaeb26567..c0904238c3 100644 --- a/cpp/autosar/test/rules/A7-3-1/test.cpp +++ b/cpp/autosar/test/rules/A7-3-1/test.cpp @@ -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 { @@ -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) @@ -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 +}; \ No newline at end of file