Skip to content
Snippets Groups Projects

Code adjustments to avoid gcc-13 warnings about dangling references

Merged Mattijs Janssens requested to merge feature-gcc13 into develop

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • assigned to @mark

  • Mark OLESEN mentioned in commit 0acffe4a

    mentioned in commit 0acffe4a

  • Mark OLESEN added 10 commits

    added 10 commits

    Compare with previous version

  • 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 as SubField<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 a tmp and that tmp can be de-referenced to obtain the valScalarField, 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 a Foam::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 temporary Foam::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!

  • Mark OLESEN marked this merge request as ready

    marked this merge request as ready

  • Mark OLESEN changed title from Draft: COMP: g++13: reference to temp. See g++-13. to Code adjustments to avoid gcc-13 warnings about dangling references

    changed title from Draft: COMP: g++13: reference to temp. See g++-13. to Code adjustments to avoid gcc-13 warnings about dangling references

  • Mark OLESEN changed the description

    changed the description

  • Mark OLESEN mentioned in commit e3f0521b

    mentioned in commit e3f0521b

  • merged

Please register or sign in to reply
Loading