Skip to content

Commit fc8fb7e

Browse files
Finalize firs draft of Rule 6-2-3
1 parent d79cf17 commit fc8fb7e

File tree

11 files changed

+177
-17
lines changed

11 files changed

+177
-17
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
- All queries using `Linkage.qll`:
2+
- The logic for determining whether a namespace is within an anonymous namespace, directly or indirectly, has been refactored.
3+
- No visible change in behavior or performance is expected.

cpp/common/src/codingstandards/cpp/Linkage.qll

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ private predicate isSpecificationVariable(Variable v) {
1717
/** Holds if `elem` has internal linkage. */
1818
predicate hasInternalLinkage(Element elem) {
1919
// An unnamed namespace or a namespace declared directly or indirectly within an unnamed namespace has internal linkage
20-
directlyOrIndirectlyUnnnamedNamespace(elem)
20+
elem instanceof WithinAnonymousNamespace
2121
or
2222
exists(Declaration decl | decl = elem |
2323
// A name having namespace scope has internal linkage if it is the name of
@@ -46,7 +46,7 @@ predicate hasInternalLinkage(Element elem) {
4646
)
4747
)
4848
or
49-
directlyOrIndirectlyUnnnamedNamespace(decl.getNamespace()) and
49+
decl.getNamespace() instanceof WithinAnonymousNamespace and
5050
inheritsLinkageOfNamespace(decl.getNamespace(), decl)
5151
or
5252
exists(Class klass |
@@ -59,12 +59,12 @@ predicate hasInternalLinkage(Element elem) {
5959
/** Holds if `elem` has external linkage. */
6060
predicate hasExternalLinkage(Element elem) {
6161
elem instanceof Namespace and
62-
not directlyOrIndirectlyUnnnamedNamespace(elem)
62+
not elem instanceof WithinAnonymousNamespace
6363
or
6464
not hasInternalLinkage(elem) and
6565
exists(Declaration decl | decl = elem |
6666
// A name having namespace scope that has not been given internal linkage above has the same linkage as the enclosing namespace if it is the name of
67-
not directlyOrIndirectlyUnnnamedNamespace(decl.getNamespace()) and
67+
not decl.getNamespace() instanceof WithinAnonymousNamespace and
6868
inheritsLinkageOfNamespace(decl.getNamespace(), decl)
6969
or
7070
exists(Class klass |
@@ -74,11 +74,11 @@ predicate hasExternalLinkage(Element elem) {
7474
)
7575
}
7676

77-
private predicate directlyOrIndirectlyUnnnamedNamespace(Namespace ns) {
78-
exists(Namespace anonymous |
79-
anonymous.isAnonymous() and
80-
ns = anonymous.getAChildNamespace*()
81-
)
77+
/**
78+
* A `Namespace` that is anonymous or indirectly contained within an unnamed namespace.
79+
*/
80+
class WithinAnonymousNamespace extends Namespace {
81+
WithinAnonymousNamespace() { getParentNamespace*().isAnonymous() }
8282
}
8383

8484
private predicate hasLinkageOfTypedef(TypedefType typedef, Element decl) {

cpp/common/src/codingstandards/cpp/exclusions/cpp/Declarations8.qll

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import codingstandards.cpp.exclusions.RuleMetadata
55

66
newtype Declarations8Query =
77
TSourceCodeImplementedOnlyOnceQuery() or
8-
TTemplateSpecializationWrongLocationQuery()
8+
TTemplateSpecializationWrongLocationQuery() or
9+
TDuplicateTypeDefinitionsQuery()
910

1011
predicate isDeclarations8QueryMetadata(Query query, string queryId, string ruleId, string category) {
1112
query =
@@ -25,6 +26,15 @@ predicate isDeclarations8QueryMetadata(Query query, string queryId, string ruleI
2526
"cpp/misra/template-specialization-wrong-location" and
2627
ruleId = "RULE-6-2-3" and
2728
category = "required"
29+
or
30+
query =
31+
// `Query` instance for the `duplicateTypeDefinitions` query
32+
Declarations8Package::duplicateTypeDefinitionsQuery() and
33+
queryId =
34+
// `@id` for the `duplicateTypeDefinitions` query
35+
"cpp/misra/duplicate-type-definitions" and
36+
ruleId = "RULE-6-2-3" and
37+
category = "required"
2838
}
2939

3040
module Declarations8Package {
@@ -41,4 +51,11 @@ module Declarations8Package {
4151
// `Query` type for `templateSpecializationWrongLocation` query
4252
TQueryCPP(TDeclarations8PackageQuery(TTemplateSpecializationWrongLocationQuery()))
4353
}
54+
55+
Query duplicateTypeDefinitionsQuery() {
56+
//autogenerate `Query` type
57+
result =
58+
// `Query` type for `duplicateTypeDefinitions` query
59+
TQueryCPP(TDeclarations8PackageQuery(TDuplicateTypeDefinitionsQuery()))
60+
}
4461
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* @id cpp/misra/duplicate-type-definitions
3+
* @name RULE-6-2-3: RULE-6-2-3: Duplicate type definitions across files
4+
* @description Defining a type with the same fully qualified name in multiple files increases the
5+
* risk of ODR violations and undefined behavior.
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity error
9+
* @tags external/misra/id/rule-6-2-3
10+
* correctness
11+
* maintainability
12+
* scope/system
13+
* external/misra/enforcement/decidable
14+
* external/misra/obligation/required
15+
*/
16+
17+
import cpp
18+
import codingstandards.cpp.misra
19+
import codingstandards.cpp.Linkage
20+
21+
class UserTypeDefinition extends TypeDeclarationEntry {
22+
UserTypeDefinition() {
23+
(isDefinition() or getDeclaration() instanceof TypedefType) and
24+
not getDeclaration().(Class).isAnonymous() and
25+
not getDeclaration().(Union).isAnonymous() and
26+
not getDeclaration().(Enum).isAnonymous() and
27+
not getDeclaration() instanceof ClassTemplateSpecialization and
28+
not getDeclaration().getNamespace() instanceof WithinAnonymousNamespace
29+
}
30+
31+
UserType getUserType() { result = getDeclaration() }
32+
}
33+
34+
from UserTypeDefinition t1, UserTypeDefinition t2
35+
where
36+
not isExcluded(t1, Declarations8Package::duplicateTypeDefinitionsQuery()) and
37+
t1.getUserType().getQualifiedName() = t2.getUserType().getQualifiedName() and
38+
t1.getFile() != t2.getFile() and
39+
t1.getFile().getAbsolutePath() < t2.getFile().getAbsolutePath() // Report only once per pair
40+
select t1, "Type '" + t1.getUserType().getQualifiedName() + "' is defined in files $@ and $@.", t1,
41+
t1.getFile().getBaseName(), t2, t2.getFile().getBaseName()

cpp/misra/src/rules/RULE-6-2-3/SourceCodeImplementedOnlyOnce.ql

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
/**
22
* @id cpp/misra/source-code-implemented-only-once
33
* @name RULE-6-2-3: The source code used to implement an entity shall appear only once
4-
* @description Implementing an entity in multiple source locations violates the one-definition rule
5-
* and leads to undefined behavior.
4+
* @description Implementing an entity in multiple source locations increases the risk of ODR
5+
* violations and undefined behavior.
66
* @kind problem
77
* @precision very-high
88
* @problem.severity error
99
* @tags external/misra/id/rule-6-2-3
1010
* correctness
11+
* maintainability
1112
* scope/system
1213
* external/misra/enforcement/decidable
1314
* external/misra/obligation/required

cpp/misra/src/rules/RULE-6-2-3/TemplateSpecializationWrongLocation.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* @problem.severity error
99
* @tags external/misra/id/rule-6-2-3
1010
* correctness
11+
* maintainability
1112
* scope/system
1213
* external/misra/enforcement/decidable
1314
* external/misra/obligation/required
@@ -24,7 +25,6 @@ predicate specializedWithFileDeclaredType(ClassTemplateSpecialization spec) {
2425
)
2526
}
2627

27-
2828
from ClassTemplateSpecialization spec, Class primaryTemplate, File primaryFile
2929
where
3030
not isExcluded(spec, Declarations8Package::templateSpecializationWrongLocationQuery()) and
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
| test.cpp:15:8:15:22 | definition of StructRedefined | Type 'StructRedefined' is defined in files $@ and $@. | test.cpp:15:8:15:22 | definition of StructRedefined | test.cpp | test2.cpp:16:8:16:22 | definition of StructRedefined | test2.cpp |
2+
| test.cpp:22:29:22:40 | definition of TplRedefined<T> | Type 'TplRedefined<T>' is defined in files $@ and $@. | test.cpp:22:29:22:40 | definition of TplRedefined<T> | test.cpp | test2.cpp:22:29:22:40 | definition of TplRedefined<T> | test2.cpp |
3+
| test.cpp:26:6:26:18 | definition of DuplicateEnum | Type 'DuplicateEnum' is defined in files $@ and $@. | test.cpp:26:6:26:18 | definition of DuplicateEnum | test.cpp | test2.cpp:25:6:25:18 | definition of DuplicateEnum | test2.cpp |
4+
| test.cpp:27:12:27:29 | definition of DuplicateEnumClass | Type 'DuplicateEnumClass' is defined in files $@ and $@. | test.cpp:27:12:27:29 | definition of DuplicateEnumClass | test.cpp | test2.cpp:26:12:26:29 | definition of DuplicateEnumClass | test2.cpp |
5+
| test.cpp:29:17:29:32 | declaration of DuplicateTypedef | Type 'DuplicateTypedef' is defined in files $@ and $@. | test.cpp:29:17:29:32 | declaration of DuplicateTypedef | test.cpp | test2.cpp:28:17:28:32 | declaration of DuplicateTypedef | test2.cpp |
6+
| test.cpp:30:7:30:20 | declaration of DuplicateUsing | Type 'DuplicateUsing' is defined in files $@ and $@. | test.cpp:30:7:30:20 | declaration of DuplicateUsing | test.cpp | test2.cpp:29:7:29:20 | declaration of DuplicateUsing | test2.cpp |
7+
| test.cpp:31:7:31:20 | definition of DuplicateUnion | Type 'DuplicateUnion' is defined in files $@ and $@. | test.cpp:31:7:31:20 | definition of DuplicateUnion | test.cpp | test2.cpp:30:7:30:20 | definition of DuplicateUnion | test2.cpp |
8+
| test.cpp:36:7:36:11 | definition of Outer | Type 'ns1::Outer' is defined in files $@ and $@. | test.cpp:36:7:36:11 | definition of Outer | test.cpp | test2.cpp:35:7:35:11 | definition of Outer | test2.cpp |
9+
| test.cpp:37:9:37:13 | definition of Inner | Type 'ns1::Outer::Inner' is defined in files $@ and $@. | test.cpp:37:9:37:13 | definition of Inner | test.cpp | test2.cpp:36:9:36:13 | definition of Inner | test2.cpp |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
rules/RULE-6-2-3/DuplicateTypeDefinitions.ql

cpp/misra/test/rules/RULE-6-2-3/test.cpp

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,40 @@ inline void func_redeclared() {} // COMPLIANT
1212
int16_t global_noninline = 0; // COMPLIANT
1313
int func_noninline() { return 42; } // COMPLIANT
1414

15-
#include "template.h"
15+
struct StructRedefined {}; // NON_COMPLIANT
16+
struct StructUnique {}; // COMPLIANT
17+
struct StructRedeclared {}; // COMPLIANT
18+
struct IncompleteStruct; // COMPLIANT
19+
struct {
20+
} anonymousStruct1; // COMPLIANT
21+
22+
template <typename T> class TplRedefined {}; // NON_COMPLIANT
23+
template <typename T> class TplUnique {}; // COMPLIANT
24+
template <typename T> class TplRedeclared {}; // COMPLIANT
25+
26+
enum DuplicateEnum {}; // NON_COMPLIANT
27+
enum class DuplicateEnumClass {}; // NON_COMPLIANT
28+
enum {} anonymousEnum1; // COMPLIANT
29+
typedef int16_t DuplicateTypedef; // NON_COMPLIANT
30+
using DuplicateUsing = int16_t; // NON_COMPLIANT
31+
union DuplicateUnion {}; // NON_COMPLIANT
32+
union {
33+
} anonymousUnion1; // COMPLIANT
34+
35+
namespace ns1 {
36+
class Outer { // NON_COMPLIANT
37+
class Inner {}; // NON_COMPLIANT
38+
};
39+
40+
namespace {
41+
class AnonymousClass {}; // COMPLIANT
42+
} // namespace
43+
} // namespace ns1
44+
45+
void f() {
46+
auto x = []() { return 42; }; // COMPLIANT
47+
}
48+
1649
#include "compliant_specialization.h"
1750
#include "noncompliant_specialization.h"
51+
#include "template.h"

cpp/misra/test/rules/RULE-6-2-3/test2.cpp

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// Note: the COMPLIANT/NON_COMPLIANT comments are here for documentary purposes;
2+
// we do not expect alerts in this file. These should be flagged in `test.cpp`.
3+
14
#include <cstdint>
25

36
inline int16_t global_redefined = 0; // NON_COMPLIANT[False negative]
@@ -8,4 +11,36 @@ inline void func_redeclared(); // COMPLIANT
811
// Violates our implementation of 6.2.1, but legal in our implementation
912
// of 6.2.3
1013
int16_t global_noninline = 0; // COMPLIANT
11-
int func_noninline() { return 42; } // COMPLIANT
14+
int func_noninline() { return 42; } // COMPLIANT
15+
16+
struct StructRedefined {}; // NON_COMPLIANT
17+
struct StructRedeclared; // COMPLIANT
18+
struct IncompleteStruct; // COMPLIANT
19+
struct {
20+
} anonymousStruct2; // COMPLIANT
21+
22+
template <typename T> class TplRedefined {}; // NON_COMPLIANT
23+
template <typename T> class TplRedeclared; // COMPLIANT
24+
25+
enum DuplicateEnum {}; // NON_COMPLIANT
26+
enum class DuplicateEnumClass {}; // NON_COMPLIANT
27+
enum {} anonymousEnum1; // COMPLIANT
28+
typedef int16_t DuplicateTypedef; // NON_COMPLIANT
29+
using DuplicateUsing = int16_t; // NON_COMPLIANT
30+
union DuplicateUnion {}; // NON_COMPLIANT
31+
union {
32+
} anonymousUnion2; // COMPLIANT
33+
34+
namespace ns1 {
35+
class Outer { // NON_COMPLIANT
36+
class Inner {}; // NON_COMPLIANT
37+
};
38+
39+
namespace {
40+
class AnonymousClass {}; // COMPLIANT
41+
} // namespace
42+
} // namespace ns1
43+
44+
void f2() {
45+
auto x = []() { return 42; }; // COMPLIANT
46+
}

0 commit comments

Comments
 (0)