integrated-application-development / integrated-application-development/sonar-delphi

Record memory should be made deterministic in constructors

Open
#149 1 comment 0 reactions 0 assignees View on GitHub
feature rule
Dominant language
Java
Stars
159
Forks
31
Avg merge
5d 4h
Merged PRs (30d)
4

Description

### Prerequisites

- [X] This rule has not already been suggested.
- [X] This should be a new rule, not an improvement to an existing rule.
- [X] This rule would be generally useful, not specific to my code or setup.

### Suggested rule title

DeterministicRecordMemory

### Rule description

This had been suggested as part of #131 but was rather made a separate rule.

Generally, a record can be initialized by assigning all its fields (accepted for rule #131). But there is one case for which this is not enough.

The default hasher for records is wrong when there are "holes" due to field alignment in non-packed records, for example an Integer and an Int64 field. The default hasher uses also the memory values within the holes, which violates the expected invariant that two values that are Equal should have the same hash.

This rule requires a stricter initialization method for record types that are not packed and have alignment "holes". Then the only proper initialization should be done by assigning the full record, to `Default(T)` for example, or to another (initialized) value.

It was suggested that erasing memory using `FillChar` or `ZeroMemory` could be ok, but I argue that using those _to initialize a record_ is bad practice (because it can evolve badly), and is always better done with `:= Default(T)`.

Thanks to #127, this rule can be restricted to constructors and `Initialize`, as is #131.

### Rationale

Using such records as dictionary keys, for example, can lead to nasty bugs if one only ensures that all fields are initialized. The unused memory should be zeroed to get consistent hashes.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.