2

I am running clang-tidy on the code below:

BinaryTree& operator=(const BinaryTree &bt){ 
    if(this == &bt){ 
        return *this;
    } 
    if(_root != nullptr) { 
        delete _root; 
    } 
    _root = new node(nullptr, bt._root->_data); 
    copy_con(_root, bt._root); 
    return *this; 
}

It gives the following error:

Error: operator=() does not handle self-assignment properly.

This is strange, since my code does handle self-assignment in the first line!

How can I fix this?

Erel Segal-Halevi
  • 33,955
  • 36
  • 114
  • 183
shani
  • 21
  • 1
  • 2
  • 1
    Your code seems to handle that clang-tidy error as described on their docs site: https://clang.llvm.org/extra/clang-tidy/checks/bugprone-unhandled-self-assignment.html – scohe001 Jun 01 '21 at 16:36
  • The issue here is not self-assignment, but exception safety. If `new` throws in this code, target will end in invalid state, loosing previous value and not getting a new one. It could be clang-tidy just giving wrong error. – SergeyA Jun 01 '21 at 16:39
  • 3
    Unrelated to the clang-tidy error. `root_ = new node(nullptr, bt._root->_data);` will be a problem if `bt._root` is `nullptr`. – R Sahu Jun 01 '21 at 16:39
  • 3
    `if(_root != nullptr)` is not needed. You can delete pointer unconditionally. – SergeyA Jun 01 '21 at 16:41
  • 1
    Unrelated: This might be a good candidate for [Copy and Swap](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) – user4581301 Jun 01 '21 at 17:20
  • 1
    In my experience, when `if(this == &bt) { return *this; }` is necessary to protected from self-assignment probably means the routine doesn't maintain object coherence & invariance correctly. (Self-assignment is unusual, so it doesn't warrant optimization this way.) – Eljay Jun 01 '21 at 18:50
  • 1
    Updated link to llvm doc: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unhandled-self-assignment.html (one from @scohe001 didn't work anymore). – Arnaud Sep 01 '22 at 20:16

0 Answers0