Solve clang-tidy warnings #427

Activity

1. benrubson

   added this to the 1.9.3 milestone on Oct 14, 2017
2. benrubson commented on Nov 2, 2017

   

   benrubson

   on Nov 2, 2017

   Last edited by benrubson

   ContributorAuthor

   More actions

   May I ask some help to correctly solve the following warnings ?
   @rfjakob, do you think you could handle them ?
   Will make the build log even more clean.
   Thank you very much 👍

   DirNode.cpp:133:7: warning: class 'RenameOp' defines a destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
   class RenameOp {
         ^
   NullCipher.cpp:50:7: warning: class 'NullKey' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
   class NullKey : public AbstractCipherKey {
         ^
   NullCipher.cpp:56:7: warning: class 'NullDestructor' defines a destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
   class NullDestructor {
         ^
   SSL_Cipher.cpp:277:7: warning: class 'SSLKey' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
   class SSLKey : public AbstractCipherKey {
         ^
   XmlReader.cpp:165:7: warning: class 'XmlNode' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
   class XmlNode : virtual public XmlValue {
         ^
   
3. benrubson commented on Nov 7, 2017

   

   benrubson

   on Nov 7, 2017

   Last edited by benrubson

   ContributorAuthor

   More actions

   OK, so just one Clang-tidy warning remaining, the tiny one above.
   Any thoughts perhaps @vgough @rfjakob ?
   We could simply disable this warning class, but perhaps it's simple and worth making the required definitions, as a best practice ?

   Link to the rule :
   http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-special-member-functions.html
4. benrubson commented on Nov 18, 2017

   

   benrubson

   on Nov 18, 2017

   ContributorAuthor

   More actions

   OK, last thing to solve before we could I think release 1.9.3.
   Thx for your help 👍
5. rfjakob commented on Dec 3, 2017

   

   rfjakob

   on Dec 3, 2017

   Collaborator

   More actions

   I'll take a look at this.
6. benrubson commented on Dec 3, 2017

   

   benrubson

   on Dec 3, 2017

   ContributorAuthor

   More actions

   Many thanks @rfjakob 👍
7. rfjakob commented on Dec 3, 2017

   

   rfjakob

   on Dec 3, 2017

   Collaborator

   More actions

   How about undefining the other operations? Like, for RenameOp

   diff --git a/encfs/DirNode.cpp b/encfs/DirNode.cpp
   index 2c6271b..7bb6a32 100644
   --- a/encfs/DirNode.cpp
   +++ b/encfs/DirNode.cpp
   @@ -142,10 +142,16 @@ class RenameOp {
        last = renameList->begin();
      }
    
   +  // copy contructor
      RenameOp(const RenameOp &src) = default;
    
   +  // destructor, defined below
      ~RenameOp();
    
   +  RenameOp(RenameOp&& other) = delete; // move constructor
   +  RenameOp& operator=(const RenameOp& other) = delete; // copy assignment
   +  RenameOp& operator=(RenameOp&& other) = delete ;// move assignment
   +
      explicit operator bool() const { return renameList != nullptr; }
    
      bool apply();

   Compiles + tests fine. Undefining used operations would result in a compile error I guess, so this would be safe?
8. benrubson commented on Dec 3, 2017

   

   benrubson

   on Dec 3, 2017

   ContributorAuthor

   More actions

   Undefining used operations would result in a compile error I guess

   I also guess, but I'm not sure at all. Perhaps @vgough could tell us ?
   Or what about the following test, adding a dummy code operation with a deleted operator, and see if compilation fails ?
9. vgough commented on Jan 17, 2018

   

   vgough

   on Jan 17, 2018

   Owner

   More actions

   Seems reasonable to me. I'd also be fine with disabling that lint check.
10. benrubson commented on Jan 17, 2018

    

    benrubson

    on Jan 17, 2018

    ContributorAuthor

    More actions

    OK, thank you @vgough.
    Let's then go with Jakob's idea 👍
    @rfjakob, would you push your work to a new PR ?
    Many thx !
11. rfjakob commented on Jan 17, 2018

    

    rfjakob

    on Jan 17, 2018

    Collaborator

    More actions

    Sorry, the copy-pasted diff above is all I have
12. benrubson commented on Jan 17, 2018

    

    benrubson

    on Jan 17, 2018

    ContributorAuthor

    More actions

    OK, I then just have pushed a change in #458 based on your idea 👍
13. benrubson

    closed this as completedin #458on Jan 23, 2018
14. hrunting

    added a commit that references this issue on Nov 11, 2022

    Squashed 'vendor/github.com/google/benchmark/' changes from 5b7683f4.…

    ...

    8e7e676