Menu

noCopyConstructor bug with multiple inheritance (with fix suggestion)

2021-03-10
2021-03-30
  • Éric Malenfant

    Éric Malenfant - 2021-03-10

    Hi,

    I think I found a bug in the noCopyConstructor and noOperatorEq checks on types with:
    - Multiple inheritance
    - One or more of the inherited types is "unknown"
    - One or more of the inherited types is known to be copyable
    The behavior depends on the order of the inherited types: When the last type in the list is known to be copyable, the check "forgets" about the "unknown" type(s) before it.

    This is illustrated by the test added by the 0001 patch attached: The first assert passes, while the second fails.

    The 0002 patch is a minimal fix for this.

    Note:
    I think that this is a "minimal" fix. Looking at how the calling code handles unknown, it seems to me that, as it is currently, it is not useful to make the distinction between "there is definitively a non-copyable base" (the function returns true) and "there is an unknown base" (unknown is true) since the emission of the error is skipped in either cases.

    I chose to keep the change minimal and avoid going down that rabbit hole right now, but it may be better to either:
    - Completely drop the unknown parameter and simply return true as soon as a non-copyable or an unknown base is found.
    - Keep the unknown parameter and use it to emit an inconclusive warning, changing the calling code to something like:if (!hasNonCopyableBase(scope, &unknown) || (unknown && (mSettings->certainty.isEnabled(Certainty::inconclusive)))) (or something equivalent and less ugly).

    Thanks

     
  • Daniel Marjamäki

    sorry for late reply.

    I wonder if you would like to create a github pull request with this update.

    Even if it is not the perfect fix I guess it is still an improvement.

     
    • Daniel Marjamäki

      I am afraid I don't volounteer to go down the rabbit hole neither right now.. but your second suggestion sounds reasonable to me. and I do not think it's too ugly.

       
  • Éric Malenfant

    Éric Malenfant - 2021-03-30

    Thanks for your reply. I can create a PR, but I don't have time now. Hopefully next week.

     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.