Code adjustments to avoid gcc-13 warnings about dangling references
Merge request reports
Activity
assigned to @mark
mentioned in commit 0acffe4a
added 10 commits
-
25867994...89cd5844 - 9 commits from branch
develop
- 0acffe4a - COMP: update to avoid some -Wdangling-reference warnings (MR !656 (merged))
-
25867994...89cd5844 - 9 commits from branch
The new gcc
-Wdangling-reference
option does a fairly impressive job of detecting places where the reference could be considered dangling.True positives
Examples:
// incorrect use of polyPatch faceCentres: const point& c1 = p.faceCentres()[facej];
The
faceCentres()
returns a sub-slice into the face centres and returns this asSubField<vector>
, which is a pointer/size span into another container. Indexing into this is OK, except that that value should be retrieved (by copy), or need to make the SubField temporary more persistent.// correct use. Copy of value const point c1 = p.faceCentres()[facej]; // correct use. Make persistent const vectorField::subField ownFaceCentres = p.faceCentres(); ... const point& c1 = ownFaceCentres[facej];
Another very typical coding error is the handling of
tmp<..>
types.
Example,// incorrect use of tmp reference const volScalarField& epsilon = turbPtr->epsilon(); // or even these const volScalarField& epsilon = turbPtr->epsilon()(); const volScalarField& epsilon = turbPtr->epsilon().cref();
Although the
epsilon()
method returns atmp
and thattmp
can be de-referenced to obtain thevalScalarField
, the actual tmp intermediate does not survive the end of the expression - thus the returned reference is to an object that is has since been destroyed.// correct use. Keep the tmp alive const tmp<volScalarField> tepsilon(turbPtr->epsilon()); const volScalarField& epsilon = tepsilon();
Caveat fixes for phaseModel methods have not yet been applied, since there is some discrepancy in the return values between two-phase and multi-phase (needs revisiting/revising).
False positives
There are a few cases when the compiler is gets confused, or cannot dive deeply enough into the code to determine that the code is actually okay. The main source of these false positives is related to HashTable lookups using
const char
keys. The "const char*" is used to create aFoam::word
(which is passed as const-ref to the temporary). After this is used to lookup and return the value, the compiler complains that the reference to the temporaryFoam::word
is possibly dangling, even although the return reference does not actually depend on it.Some strangeness also appears to be associated with the depth that the compiler explores. In the existing code (2312 and earlier), there is the following convenience method:
// cloud.H template<class Type> static const IOField<Type>& lookupIOField(const word& fieldName, const objectRegistry& obr) { return obr.lookupObject<IOField<Type>>(fieldName); }
The compiler then warns about a possible dangling reference when this code is used:
const auto& cloud::lookupIOField<label>("origId", obr); // or even const auto& cloud::lookupIOField<label>(word("origId"), obr);
However, by providing an additional convenience method:
// cloud.H template<class Type> static const IOField<Type>& lookupIOField(const char* fieldName, const objectRegistry& obr) { return obr.lookupObject<IOField<Type>>(word(fieldName)); }
The compiler suddenly thinks that it is okay (no dangling reference), even although there is no real difference!
mentioned in commit e3f0521b