Clang issue445#506
Conversation
…ter with bounds expr in a checked scope
dtarditi
left a comment
dtarditi
left a comment
There was a problem hiding this comment.
Could you cover these cases for members of struct types also?
Please add some tests for the new cases to your pull request for the checkedc repo.
Thanks,
David
dtarditi
left a comment
dtarditi
left a comment
There was a problem hiding this comment.
Nice! I have some formatting suggestions.
| enum CheckedValueKind { | ||
| NoCheckedValue, | ||
| HasCheckedValue, | ||
| HasUnCheckedPointer |
There was a problem hiding this comment.
Could you change the case of the C in HasUnCheckedPointer to lower case (HasUncheckedPointer)? This matches the capitalization in the rest of the code base.
| } | ||
| } | ||
|
|
||
| // containsCheckedValue - check whether a field type is a checked type or is a |
There was a problem hiding this comment.
This comment needs to be updated.
| // This is a valid initialization value, so we don't have to issue an | ||
| // error message for them. | ||
| if (!Var->isInvalidDecl() && Var->hasLocalStorage() && | ||
| if (!Var->isInvalidDecl() && Var->hasLocalStorage() && |
There was a problem hiding this comment.
This added a space at the end of the line. Could you remove the space?
| return Type::NoCheckedValue; | ||
|
|
||
| Type::CheckedValueKind hasCheckedField = Type::NoCheckedValue; | ||
| // if this is a struct/union type, iterate all its members |
There was a problem hiding this comment.
Wording suggestion: iterate all -> iterate over all
| return; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Could you remove this white space change?
dtarditi
left a comment
dtarditi
left a comment
There was a problem hiding this comment.
Looks good. I think the wording in error messages related to integers needs to be clarified.
|
|
||
| def err_initializer_expected_for_ptr : Error< | ||
| "automatic variable %0 with _Ptr type must have initializer">; | ||
|
|
There was a problem hiding this comment.
Please use the name integer in place of int in the error messages.
The code for deciding if a function should get an itype was duplicated for function declarations and function pointer types. The function pointer version of the code had a bug in it that caused issue #498. The duplicated code has been extracted into a pair of functions that are reused for functions and function pointers. A lot of the lines changed in this PR are caused by an EnvironmentMap& parameter being changed to Constraints&. This lets the function pointer code call solutionEqualTo which is needed for correct itype insertion.
2 participants
Add features for handling two additional cases:
Testing passed:
DevTest Release Linux X64 LNT testing
DevTest Debug X64 Linux
DevTest Debug X64 Windows