diff --git a/change_notes/2025-01-21-a7-1-2-remove-function-constexpr.md b/change_notes/2025-01-21-a7-1-2-remove-function-constexpr.md new file mode 100644 index 0000000000..ac9964adc9 --- /dev/null +++ b/change_notes/2025-01-21-a7-1-2-remove-function-constexpr.md @@ -0,0 +1,2 @@ + - `A7-1-2` - `FunctionMissingConstexpr.ql` + - Address false positives by removing the query - the rule is not intended to cover functions. \ No newline at end of file diff --git a/cpp/autosar/src/rules/A7-1-2/FunctionMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/FunctionMissingConstexpr.ql deleted file mode 100644 index 1cd68447fe..0000000000 --- a/cpp/autosar/src/rules/A7-1-2/FunctionMissingConstexpr.ql +++ /dev/null @@ -1,160 +0,0 @@ -/** - * @id cpp/autosar/function-missing-constexpr - * @name A7-1-2: The constexpr specifier shall be used for functions whose return value can be determined at compile time - * @description Using 'constexpr' makes it clear that a function is intended to return a compile - * time constant. - * @kind problem - * @precision high - * @problem.severity recommendation - * @tags external/autosar/id/a7-1-2 - * maintainability - * external/autosar/allocated-target/implementation - * external/autosar/enforcement/automated - * external/autosar/obligation/required - */ - -import cpp -import codingstandards.cpp.autosar -import codingstandards.cpp.TrivialType - -/** Gets a non-variant member field. */ -Field getANonVariantField(Class c) { - result = c.getAField() and - result.isInitializable() and - not result instanceof AnonymousUnionField -} - -/** - * A `Field` holding an "anonymous union" as described by `[class.union]`. - * - * For example, the union in this example: - * ``` - * class C { - * union { - * int x; - * short y; - * }; - * } - * ``` - */ -class AnonymousUnionField extends Field { - AnonymousUnionField() { hasName("(unknown field)") } - - /** - * Get a direct or indirect union member. - * - * Indirect members can come from nested anonymous unions. - */ - Field getAVariantMember() { - exists(Field f | f = getType().(Union).getAField() | - if f instanceof AnonymousUnionField - then result = f.(AnonymousUnionField).getAVariantMember() - else result = f - ) - } - - /** - * Holds if one variant member of this anonymous union field is initialized using NSDMI. - */ - predicate isExplicitlyInitialized() { exists(getAVariantMember().getInitializer().getExpr()) } -} - -/** - * Get a union which is not initialized by NSDMI. - */ -AnonymousUnionField getAnUninitializedAnonymousUnionField(Class c) { - result = c.getAField() and - not result.isExplicitlyInitialized() -} - -/** - * A function that can be `constexpr` specified according to the constraints for a `constexpr` - * function as specified in `[dcl.constexpr]/3`. - */ -class EffectivelyConstExprFunction extends Function { - EffectivelyConstExprFunction() { - // Not already marked as constexpr - not isDeclaredConstexpr() and - // Not virtual - not isVirtual() and - // Returns a literal type (which can be 'void') - (isLiteralType(getType()) or this instanceof Constructor) and - // Exclude cases that shouldn't be const or can't be const - not this instanceof Destructor and - not this instanceof CopyAssignmentOperator and - not this instanceof MoveAssignmentOperator and - not this.isCompilerGenerated() and - // All parameters are literal types - forall(Parameter p | p = getAParameter() | isLiteralType(p.getType())) and - // The function body is either deleted, defaulted or does not include one of the precluding - // statement kinds and is both side-effect free and created by the user - ( - isDeleted() - or - isDefaulted() - or - not this = any(AsmStmt a).getEnclosingFunction() and - not this = any(GotoStmt g).getEnclosingFunction() and - not this = any(TryStmt t).getEnclosingFunction() and - not exists(LocalVariable lv | this = lv.getFunction() | - not isLiteralType(lv.getType()) - or - lv instanceof StaticStorageDurationVariable - or - lv.isThreadLocal() - or - not exists(lv.getInitializer().getExpr()) - ) and - // For `constexpr` functions, the compiler only checks the rules above - it doesn't check - // whether the function can be evaluated as a compile time constant until the function is used, - // and then only confirms that it evaluates to a compile-time constant for a specific set of - // arguments used in another constexpr calculation. We approximate this by identifying the set - // of functions that are (conservatively) side-effect free. - isSideEffectFree() and - // "User defined" in some way - hasDefinition() and - not isCompilerGenerated() - ) and - ( - // A constructor should satisfy the constraints as specified in `[dcl.constexpr]/4`. - this instanceof Constructor - implies - ( - // No virtual base class - not getDeclaringType().getDerivation(_).isVirtual() and - ( - // All non-variant members initialized by this constructor - forall(Field f | f = getANonVariantField(getDeclaringType()) | - exists(ConstructorFieldInit cfi | - // Even if this field has a `getInitializer()` a `ConstructorFieldInit` will also be - // present on each constructor - cfi.getEnclosingFunction() = this and cfi.getTarget() = f - ) - ) and - // At least one variant member is initialized for each `AnonymousUnionField` which is not - // initialized with a `Field.getInitializer()`. This is different to the non-variant - // member case above - forall(AnonymousUnionField f | - f = getAnUninitializedAnonymousUnionField(getDeclaringType()) - | - exists(ConstructorFieldInit cfi | - cfi.getEnclosingFunction() = this and cfi.getTarget() = f.getAVariantMember() - ) - ) - or - // The function is deleted or defaulted, and every field has an NSDMI, and there are no - // uninitialized anonymous union fields - (isDeleted() or isDefaulted()) and - forall(Field f | f = getANonVariantField(getDeclaringType()) | - exists(f.getInitializer().getExpr()) - ) and - not exists(getAnUninitializedAnonymousUnionField(getDeclaringType())) - ) - ) - ) - } -} - -from EffectivelyConstExprFunction ecef -where not isExcluded(ecef, ConstPackage::functionMissingConstexprQuery()) -select ecef, ecef.getName() + " function could be marked as 'constexpr'." diff --git a/cpp/autosar/test/rules/A7-1-2/FunctionMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/FunctionMissingConstexpr.expected deleted file mode 100644 index a6de3fd724..0000000000 --- a/cpp/autosar/test/rules/A7-1-2/FunctionMissingConstexpr.expected +++ /dev/null @@ -1,16 +0,0 @@ -| test.cpp:30:3:30:17 | NonLiteralClass | NonLiteralClass function could be marked as 'constexpr'. | -| test.cpp:59:5:59:6 | h1 | h1 function could be marked as 'constexpr'. | -| test.cpp:67:5:67:6 | h2 | h2 function could be marked as 'constexpr'. | -| test.cpp:100:5:100:6 | h8 | h8 function could be marked as 'constexpr'. | -| test.cpp:117:7:117:9 | mf1 | mf1 function could be marked as 'constexpr'. | -| test.cpp:126:3:126:23 | MissingConstexprClass | MissingConstexprClass function could be marked as 'constexpr'. | -| test.cpp:127:3:127:23 | MissingConstexprClass | MissingConstexprClass function could be marked as 'constexpr'. | -| test.cpp:128:3:128:23 | MissingConstexprClass | MissingConstexprClass function could be marked as 'constexpr'. | -| test.cpp:161:3:161:26 | VariantMemberInitialized | VariantMemberInitialized function could be marked as 'constexpr'. | -| test.cpp:162:3:162:26 | VariantMemberInitialized | VariantMemberInitialized function could be marked as 'constexpr'. | -| test.cpp:163:3:163:26 | VariantMemberInitialized | VariantMemberInitialized function could be marked as 'constexpr'. | -| test.cpp:190:3:190:22 | VariantMemberNotInit | VariantMemberNotInit function could be marked as 'constexpr'. | -| test.cpp:269:26:269:26 | init | init function could be marked as 'constexpr'. | -| test.cpp:269:26:269:29 | init | init function could be marked as 'constexpr'. | -| test.cpp:271:26:271:26 | init | init function could be marked as 'constexpr'. | -| test.cpp:277:6:277:32 | test_template_instantiation | test_template_instantiation function could be marked as 'constexpr'. | diff --git a/cpp/autosar/test/rules/A7-1-2/FunctionMissingConstexpr.qlref b/cpp/autosar/test/rules/A7-1-2/FunctionMissingConstexpr.qlref deleted file mode 100644 index 723a910948..0000000000 --- a/cpp/autosar/test/rules/A7-1-2/FunctionMissingConstexpr.qlref +++ /dev/null @@ -1 +0,0 @@ -rules/A7-1-2/FunctionMissingConstexpr.ql \ No newline at end of file diff --git a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected index 31c26a11ff..5feec712b8 100644 --- a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected +++ b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected @@ -8,16 +8,17 @@ | test.cpp:44:16:44:17 | lc | Variable 'lc' could be marked 'constexpr'. | | test.cpp:45:17:45:19 | lc2 | Variable 'lc2' could be marked 'constexpr'. | | test.cpp:55:7:55:8 | m2 | Variable 'm2' could be marked 'constexpr' and static. | -| test.cpp:130:7:130:8 | m1 | Variable 'm1' could be marked 'constexpr' and static. | -| test.cpp:141:7:141:8 | m1 | Variable 'm1' could be marked 'constexpr' and static. | -| test.cpp:221:7:221:8 | l1 | Variable 'l1' could be marked 'constexpr'. | -| test.cpp:235:7:235:8 | l6 | Variable 'l6' could be marked 'constexpr'. | -| test.cpp:237:7:237:8 | l8 | Variable 'l8' could be marked 'constexpr'. | -| test.cpp:240:7:240:9 | l10 | Variable 'l10' could be marked 'constexpr'. | -| test.cpp:243:7:243:9 | l12 | Variable 'l12' could be marked 'constexpr'. | -| test.cpp:248:7:248:9 | l15 | Variable 'l15' could be marked 'constexpr'. | -| test.cpp:250:7:250:9 | l16 | Variable 'l16' could be marked 'constexpr'. | -| test.cpp:251:7:251:9 | l17 | Variable 'l17' could be marked 'constexpr'. | -| test.cpp:257:7:257:9 | l21 | Variable 'l21' could be marked 'constexpr'. | -| test.cpp:262:7:262:9 | l24 | Variable 'l24' could be marked 'constexpr'. | -| test.cpp:263:7:263:9 | l25 | Variable 'l25' could be marked 'constexpr'. | +| test.cpp:65:7:65:8 | x2 | Variable 'x2' could be marked 'constexpr'. | +| test.cpp:66:13:66:14 | x3 | Variable 'x3' could be marked 'constexpr'. | +| test.cpp:76:7:76:8 | m1 | Variable 'm1' could be marked 'constexpr' and static. | +| test.cpp:91:7:91:8 | l1 | Variable 'l1' could be marked 'constexpr'. | +| test.cpp:105:7:105:8 | l6 | Variable 'l6' could be marked 'constexpr'. | +| test.cpp:107:7:107:8 | l8 | Variable 'l8' could be marked 'constexpr'. | +| test.cpp:110:7:110:9 | l10 | Variable 'l10' could be marked 'constexpr'. | +| test.cpp:113:7:113:9 | l12 | Variable 'l12' could be marked 'constexpr'. | +| test.cpp:118:7:118:9 | l15 | Variable 'l15' could be marked 'constexpr'. | +| test.cpp:120:7:120:9 | l16 | Variable 'l16' could be marked 'constexpr'. | +| test.cpp:121:7:121:9 | l17 | Variable 'l17' could be marked 'constexpr'. | +| test.cpp:127:7:127:9 | l21 | Variable 'l21' could be marked 'constexpr'. | +| test.cpp:132:7:132:9 | l24 | Variable 'l24' could be marked 'constexpr'. | +| test.cpp:133:7:133:9 | l25 | Variable 'l25' could be marked 'constexpr'. | diff --git a/cpp/autosar/test/rules/A7-1-2/test.cpp b/cpp/autosar/test/rules/A7-1-2/test.cpp index 664a9cb8e7..5366a59f95 100644 --- a/cpp/autosar/test/rules/A7-1-2/test.cpp +++ b/cpp/autosar/test/rules/A7-1-2/test.cpp @@ -56,71 +56,17 @@ class MemberConstExpr { int m3 = 0; // COMPLIANT - can be set by constructor }; -int h1(int x, int y) { // NON_COMPLIANT - return x + y; -} - -constexpr int h1_correct(int x, int y) { // COMPLIANT - return x + y; -} - -int h2(int x) { return h1(x, 1) + 1; } // NON_COMPLIANT -constexpr int h2_correct(int x) { return h1_correct(x, 1) + 1; } // COMPLIANT - -int h3(int x) { // COMPLIANT - uses goto, so can't be constexpr - if (x) { - goto l1; - } else { - return 10; - } -l1: - return 1; -} - -int h4(int x) { // COMPLIANT - uses try, so can't be constexpr - try { - return 1; - } catch (...) { - } -} - -int h5(int x) { // COMPLIANT - declares non literal local var - NonLiteralClass nlc; -} - -int h6(int x) { // COMPLIANT - declares static variable - static int i = x; - return x; -} - -int h7(int x) { // COMPLIANT - declares no init variable - int i; -} +int h1(int x, int y) { return x + y; } -int h8(int x) { // NON_COMPLIANT - could be constexpr - int i = x; - return i; -} +constexpr int h1_const(int x, int y) { return x + y; } -constexpr int h8_correct(int x) { // COMPLIANT - int i = x; - return i; +int h2() { + int x1 = h1(1, 1); // COMPLIANT + int x2 = h1_const(1, 1); // NON_COMPLIANT + const int x3 = h1_const(1, 1); // NON_COMPLIANT + constexpr int x4 = h1_const(1, 1); // COMPLIANT } -int h9(int x) { // COMPLIANT - declares thread local variable - thread_local int i = x; - return x; -} - -class ConstexprFunctionClass { -public: - int mf1(int x) { return m1 + x; } // NON_COMPLIANT - constexpr int mf1_correct(int x) { return m1 + x; } // COMPLIANT - -private: - int m1; -}; - class MissingConstexprClass { public: MissingConstexprClass() = default; // NON_COMPLIANT @@ -130,82 +76,6 @@ class MissingConstexprClass { int m1 = 0; // NON_COMPLIANT }; -class VirtualBaseClass {}; - -class DerivedClass : public virtual VirtualBaseClass { -public: - DerivedClass() = default; // COMPLIANT - DerivedClass(int i) = delete; // COMPLIANT - DerivedClass(int i, LiteralClass lc) {} // COMPLIANT -private: - int m1 = 0; // NON_COMPLIANT -}; - -class NotAllMembersInitializedClass { -public: - NotAllMembersInitializedClass() = default; // COMPLIANT - NotAllMembersInitializedClass(int i) = delete; // COMPLIANT - NotAllMembersInitializedClass(int i, LiteralClass lc) {} // COMPLIANT -private: - int m1; -}; - -class NonLiteralParamsClass { -public: - NonLiteralParamsClass(int i, NonLiteralClass lc) {} // COMPLIANT -}; - -// Variant members are always initialized, so this can be marked constexpr -class VariantMemberInitialized { -public: - VariantMemberInitialized() = default; // NON_COMPLIANT - VariantMemberInitialized(int i) = delete; // NON_COMPLIANT - VariantMemberInitialized(int i, LiteralClass lc) {} // NON_COMPLIANT -private: - union { - int i = 0; - short s; - }; -}; - -class VariantMemberInitConstexpr { -public: - constexpr VariantMemberInitConstexpr() = default; // COMPLIANT - constexpr VariantMemberInitConstexpr(int i) = delete; // COMPLIANT - constexpr VariantMemberInitConstexpr(int i, LiteralClass lc) {} // COMPLIANT -private: - union { - int i = 0; - short s; - }; -}; - -// Variant members are not initialized at declaration, so we can only mark the -// constructors as constexpr if we explicitly initialize the variant member -class VariantMemberNotInit { -public: - VariantMemberNotInit() = default; // COMPLIANT - VariantMemberNotInit(int pi) = delete; // COMPLIANT - VariantMemberNotInit(int pi, LiteralClass lc) {} // COMPLIANT - VariantMemberNotInit(LiteralClass lc, int pi) : i(pi) {} // NON_COMPLIANT - constexpr VariantMemberNotInit(LiteralClass lc, short pi) // COMPLIANT - : i(pi) {} - -private: - union { - int i; - short s; - }; -}; - -class ExcludedCases { -public: - ~ExcludedCases() {} // COMPLIANT - - void operator=(ExcludedCases &) {} // COMPLIANT - void operator=(ExcludedCases &&) {} // COMPLIANT -}; - extern int random(); constexpr int add(int x, int y) { return x + y; } // Example with compile time constant literal value as default argument diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Const.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Const.qll index 09f40388cc..f542ddf486 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Const.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Const.qll @@ -7,7 +7,6 @@ newtype ConstQuery = TRemoveConstOrVolatileQualificationAutosarQuery() or TDeclarationUnmodifiedObjectMissingConstSpecifierQuery() or TVariableMissingConstexprQuery() or - TFunctionMissingConstexprQuery() or TCvQualifiersNotPlacedOnTheRightHandSideQuery() or TOutputParametersUsedQuery() or TInOutParametersDeclaredAsTNotModifiedQuery() or @@ -45,15 +44,6 @@ predicate isConstQueryMetadata(Query query, string queryId, string ruleId, strin ruleId = "A7-1-2" and category = "required" or - query = - // `Query` instance for the `functionMissingConstexpr` query - ConstPackage::functionMissingConstexprQuery() and - queryId = - // `@id` for the `functionMissingConstexpr` query - "cpp/autosar/function-missing-constexpr" and - ruleId = "A7-1-2" and - category = "required" - or query = // `Query` instance for the `cvQualifiersNotPlacedOnTheRightHandSide` query ConstPackage::cvQualifiersNotPlacedOnTheRightHandSideQuery() and @@ -149,13 +139,6 @@ module ConstPackage { TQueryCPP(TConstPackageQuery(TVariableMissingConstexprQuery())) } - Query functionMissingConstexprQuery() { - //autogenerate `Query` type - result = - // `Query` type for `functionMissingConstexpr` query - TQueryCPP(TConstPackageQuery(TFunctionMissingConstexprQuery())) - } - Query cvQualifiersNotPlacedOnTheRightHandSideQuery() { //autogenerate `Query` type result = diff --git a/rule_packages/cpp/Const.json b/rule_packages/cpp/Const.json index c574e547bf..55c5ed6f90 100644 --- a/rule_packages/cpp/Const.json +++ b/rule_packages/cpp/Const.json @@ -71,17 +71,6 @@ "tags": [ "maintainability" ] - }, - { - "description": "Using 'constexpr' makes it clear that a function is intended to return a compile time constant.", - "kind": "problem", - "name": "The constexpr specifier shall be used for functions whose return value can be determined at compile time", - "precision": "high", - "severity": "recommendation", - "short_name": "FunctionMissingConstexpr", - "tags": [ - "maintainability" - ] } ], "title": "The constexpr specifier shall be used for values that can be determined at compile time."