MISRA rule 9.2 The initializer for an aggregate or union shall be enclosed in braces#2899
Conversation
| def getArrayDimensionsAndValueType(token): | ||
| dimensions = [] | ||
| while token and token.str == '[': | ||
| op1Value = getIntvalue(token.astOperand1) |
There was a problem hiding this comment.
hmm.. do we need to handle variable length arrays?
void foo(int x) {
if (x == 10 || x == 100) {
int arr[x]; // <- x has two values 10 and 100.
}
}
There was a problem hiding this comment.
For 9.2 we don't really use the length, since 9.2 only cares about structure. 9.3 and 9.4, however, will have to.
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
nice work! Some small nits..
| dimensions = None | ||
| break | ||
|
|
||
| if token.valueType: |
There was a problem hiding this comment.
as far as I see this code
if token.valueType:
valueType = token.valueType
else:
valueType = None
is equivalent with:
valueType = token.valueType
There was a problem hiding this comment.
Agree. I believe the nullcheck was supposed to be for the token.
| self.valueType = valueType | ||
| self.dimensions = dimensions | ||
|
|
||
| def getIntvalue(token): |
There was a problem hiding this comment.
maybe it does not matter much now.. but this function seems to be a bit random to me. If it is used for instance on a variable that can be either 10 or 100 then one of those values will be randomly returned.
There was a problem hiding this comment.
I added a function token.getKnownIntValue(). Would it be possible to use that instead?
There was a problem hiding this comment.
Refactored using this function instead, and actually caught another 9.2 violation in the misra_18_8 test.
| # Returns a list of the struct elements as StructElementDef in the order they are declared. | ||
| def getRecordElements(valueType): | ||
| elements = [] | ||
| for token in data.tokenlist: |
There was a problem hiding this comment.
hmm.. this is not how you should do it. This would have been better:
for var in valueType.typeScope.varlist:
token = var.nameToken
....
But I am a bit confused .. for some reason the Scope does not seem to have a varlist. However the dump file has the varlist info needed. So somebody should add a varlist in the Scope class.
There was a problem hiding this comment.
For this testcode:
struct A {
int x;
int y;
};
The dump file will contain:
<scope id="0x560e78458260" type="Struct" className="A" bodyStart="0x560e784532b0" bodyEnd="0x560e78456020" nestedIn="0x560e78458040">
<varlist>
<var id="0x560e784567a0"/>
<var id="0x560e78457c10"/>
</varlist>
</scope>
There was a problem hiding this comment.
We saw the varlist in the dump file, but also realized that it wasn't present in the Scope objects. So we then tried looping though the variables list, but found they weren't always in order, so we resigned to using the tokenList for now.
There was a problem hiding this comment.
ok.
I have the feeling that variables in <variables> might not be ordered always because the variables are created in a few steps.
However I am spontaneously surprised if <varlist> is not ordered.
To be clear.. you saw that <varlist> was not ordered? do you have an example code?
There was a problem hiding this comment.
I have added Scope.varlist. Can you please try if this works:
for var in valueType.typeScope.varlist:
token = var.nameToken
....
There was a problem hiding this comment.
Nice, this feels better. I've changed our code to use this new list.
Btw: It was the data.variables list that was unordered. Scope.varlist has behaved very orderly thus far
| variable = token.variable | ||
| nameToken = variable.nameToken | ||
| # Find declarations (also if isSplittedVarDeclEq is True) | ||
| if nameToken.file == token.file and nameToken.linenr == token.linenr and nameToken.column == token.column: |
There was a problem hiding this comment.
I suggest nameToken is token
There was a problem hiding this comment.
We needed to find the nameToken used in the initialization assignment when the declaration and assignment was splitted in the ast, as we've seen happen for structs and unions. I've changed this now to.
if nameToken.next.isSplittedVarDeclEq:
nameToken = nameToken.next.next
if nameToken is token:
Can we rely on this approach to always produce the desired result? It seems correct from what I can see from the dump file.
There was a problem hiding this comment.
Yes I believe we can rely on that approach.
| return True | ||
|
|
||
| # ------ | ||
| for token in data.tokenlist: |
There was a problem hiding this comment.
You can loop through each variable instead..
for var in data.variables:
... check if initialization is proper ...
danmar
left a comment
danmar
left a comment
There was a problem hiding this comment.
when CI is green I think this can be merged.
4 participants
It should be possible to reuse some of this code for 9.3, 9.4 and full support for 9.5.