LoginSignup
0
1

More than 1 year has passed since last update.

Effective Modern C++の内容をコードレビュー自動化ツールSiderで試してみる

Last updated at Posted at 2021-08-29

はじめに

Siderとは何か、何を検出できるかを述べていく。
Siderでは検出できない文法やプロジェクト特有の決め事に対してどう対応するかも述べる。
Effective Modern C++の未記載章に関しては学習が進捗次第更新予定。

感想

自動チェックツールとしては十分良いと感じた。
visual studioではワーニング、エラーとして検知できないものも検知できることもある。
GoodCheckと組み合わせれば独自のルールに対しても対応できるのでよさそう。

Siderとは

リンク:Sider
・コードチェックを自動化してくれるサービスのこと。
・プッシュ、プルリクエスト時に解析して問題、複雑度などを解析、通知してくれる
・類似ロジックの一部変更にも対応

と、色々なサイトに上記のようなことが書かれているが、実際の性能はどうなのか?
Effective Modern C++を学習していて気を付けるべきだな、自動チェックしてくれたらいいなと思った点をSiderでチェックできるかを試してみた。

1章 型推論 2章 auto

autoで注意すべき点
①初期化漏れ
②右辺値左辺値間違い
③明示的な型変換を使わないと正しく推論できないケース

auto.cpp

#include <iostream>
#include <vector>


std::vector<bool> f(std::vector<bool>& a) {

    a[5] = 1;

    return a;
}

void function(int&) {

}

int main(void)
{
    //未初期化
    auto a;

    //左辺値に右辺値を入れてみる
    function(10);

    //プロクシクラスに推論され、文末でライフタイムが切れて未定義な場合
    std::vector<bool> a;
    a.resize(10);
    auto c = f(a)[5];
    std::cout << c;

}

③に対してはエラーが出なかった。

image.png

3章 現代のC++への移行

項目7:オブジェクト作成時の()と{}の違い

()と{}での初期化のメリットデメリットは以下のようになっている。
image.png

検知したいこととしては以下
①()による暗黙の型変換による精度落ち(ビルド時ワーニング出るので正直問題なし)
②デフォルトコンストラクタがないときの()による意図しない関数宣言
③{}で初期化した時にstd::initializer_listでコンストラクタがオーバーロードされる。特にvector。

initialize.cpp

class Widget {
public:
    //デフォルトのコンストラクタはない状態
    Widget(int arg) :w(arg) {};
    int w;



private:
    int x{ 0 };
    int y = 0;
    int z(0);//ビルドエラー

};

class Widget2 
{
public:
    Widget2() {};


    Widget2(double arg)
    {
        std::cout << "double constructer";
    };


    std::vector<int>v;
    Widget2(std::initializer_list<int> list)
    {
        std::cout << "initializer_list";
        for (auto i : list) {
            v.push_back(i);
        }

    }

};

int main(void)
{
    //精度が落ちる変換の初期化
    double x = 1.0;
    double y = 1.0;
    int sum{ x + y };//{}ならビルドエラー。精度落ち許容しない
    int sum2( x + y );//()は精度低下許容



    //デフォルトのコンストラクタは存在しない時に()で初期化
    Widget w1(1);
    Widget w2();//関数定義になってしまっている。デフォルトのコンストラクタは存在しない。ビルドエラーなし
    Widget w3{};//ビルドエラーになる
    Widget w4{1};//OK

    //{}初期化ではinitialize_listが優先的に呼び出される
    Widget2 w5(10.0); //()では型のマッチが優先される
    Widget2 w6{ 10.0 }; //{}ではinitializer_listが優先され,精度落ちでビルドエラー
    Widget2 w7({});//空のinitializer_listを渡す
    Widget2 w8{ {} };//空のinitializer_listを渡す
    std::vector<int> a(10, 20);//値20が10個
    std::vector<int> b{ 10,20 };//initializer_listにより{10,20}が生成.std::vectorのコンストラクタ設計がいけてない

    std::unique_ptr<int[]> p1 = std::make_unique<int[]>(3);//要素3つ
    std::unique_ptr<int[]> p2 = std::make_unique<int[]>{ 3 };//()でしか初期化できないようになっている。ビルドエラー
    std::shared_ptr<int[]> p3 = std::make_shared<int[]>( 3 );
    std::shared_ptr<int[]> p4 = std::make_shared<int[]>{ 3 };//()でしか初期化できないようになっている。ビルドエラー
}

①精度落ち検出可能
image.png

②ビルド時は検出できなかったが、ワーニングがSiderでは出力されていた
image.png

③vectorの場合はワーニングなどはなかったが、std::make_unique と std::make_sharedに対してはエラーが表示された。(ビルド時にも表示されてはいるのでSiderで確かめる必要はないが)
image.png

項目8:0やNULLよりnullptrを優先する

注意すべき点
・0やNULLはポインタ型ではない。nullptrはすべてのポインタ型に変換可能なため、nullptrを優先する
・汎整数型とポインタ型のオーバーロードは避ける

image.png

NULLの場合はあいまいとエラーになっていいる。
image.png

項目9:typedef よりもエイリアス宣言を優先する

エイリアス宣言を使いましょうということだけなので省略

項目 10:enum にはスコープを設ける

注意事項
C++98スタイルのenumはスコープを持たないenumであるため、①名前空間の汚染と②暗黙の型変換が発生してしまう。
ビルド時は①はワーニングが表示されるが、②はなにも表示されない。仕方ない気もするが。

enum.cpp
#include <iostream>
#include <vector>
#include <tuple>

enum Color 
{
    black,
    white,
    red
};


enum class Color2
{
    black,
    white,
    red
};


void func(double d) {

}


enum UserInfoFields { uiName, uiEmail, uiReputation };

template<typename E> // C++14
constexpr std::underlying_type_t<E>
toUType(E enumerator) noexcept
{
    return static_cast<std::underlying_type_t<E>>(enumerator);
}


int main(void)
{
    auto white = false;//本書と異なりビルド可能。ビルド時にワーニングは表示されている
    func(white);//暗黙の型変換発生

    //Color  c = white;

    Color2  c2 = Color2::white;
    func(c2);//暗黙の型変換は発生しない。キャスト必要。

    using UserInfo = // type alias; see Item 9
        std::tuple<std::string, // name
        std::string, // email
        std::size_t>;

    UserInfo uInfo;
    auto val = std::get<uiName>(uInfo);//UserInfoFieldsからstd::size_tへの暗黙の変換のおかげ




}

enum classの暗黙の型変換エラーはビルド時同様に検出可能。注意したかった①と②に関しては検出できていない。
image.png

項目 11:未定義 private 関数よりも =delete を優先する

ビルド時にエラーが表示されるため、Siderで検出できるか試す必要なさそうだが、Sider側でも検出できているか検証
①deleteされた関数に対する呼び出しをエラーにできているか。
②deleteをprivateにした時とpublicに定義した時のワーニングの違い
ビルド時はpublicの方が分かりやすいエラー表示になっている。
image.png

delete.cpp

class Widget
{

public:
    Widget() {};
    Widget(const Widget&) = delete;
    Widget& operator=(const Widget&) = delete;
};

class Widget2
{
public:
    Widget2() {};

private:
    Widget2(const Widget2&) = delete;
    Widget2& operator=(const Widget2&) = delete;
};


template<typename T>
void processPointer(T* ptr) {};

template<>
void processPointer<double>(double*) = delete;

int main(void)
{
    //deleteをpublicに
    Widget tmp;
    Widget tmp2 = tmp;//「削除された関数を参照」というワーニングで分かりやすい

    //deleteをprivateに
    Widget2 tmp3;
    Widget2 tmp4 = tmp3;//「アクセスできません」というワーニングで分かりにくい

    int a = 10;
    processPointer<int>(&a);

    double b = 1.0;
    processPointer<char>(&b);
}

Siderだとprivateでもpublicでもどちらもdeleteされたコンストラクタだと表示され分かりやすい。
image.png

項目 12:オーバライドする関数は override と宣言する

下記の条件を満たさないとオーバーライドできない。
virtualがついていない、関数名が異なる、引数の型が違う、戻り値の型が違う、const性が一致していない、戻り値の型が違う、関数の参照修飾子が一致している必要がある。

ビルド時にエラーやワーニングは出ないが、Siderはどうか?
→Siderも出ない。
→ovverideをつけてワーニングに表示されるようにする必要がある。

delete.cpp

#include <iostream>
#include <vector>

class Base {
public:
    virtual void mf1() const;
    virtual void mf2(int x);
    virtual void mf3()&;
    void mf4() const;
};
class Derived : public Base {
public:
    virtual void mf1();
    virtual void mf2(unsigned int x);
    virtual void mf3()&&;
    void mf4() const;
};


int main(void)
{

}

おまけの話
参照修飾子をつけると左辺値オブジェクト、右辺値オブジェクトを区別できる。
ビルド時もSiderでも指摘が出る。

delete.cpp
#include <iostream>
#include <vector>

class Widget {
public:
    using DataType = std::vector<double>; 

    //*thisが左辺値の場合に使用可能
    DataType& data() &
    {
        std::cout << "左辺値参照";
        return values;
    }

    //*thisが右辺値の場合に使用可能
    //DataType data() &&
    //{
    //  std::cout << "右辺値参照";
    //  return std::move(values);
    //}

private:
    DataType values;
};

class Base {
public:
    virtual void mf1() const;
    virtual void mf2(int x);
    virtual void mf3()&;
    virtual void mf4() const;
    virtual void mf5()&&;
};
class Derived : public Base {
public:
    virtual void mf1()const override;
    virtual void mf2(int x) override;

    virtual void mf3() & override
    {

    };

    virtual void mf4() const override;

    virtual void mf5() && override {

    };
};

Widget makeWidget() {

    Widget* tmp = new Widget;

    return *tmp;
}

int main(void)
{
    Widget w;

    auto vals1 = w.data();

    auto vals2 = makeWidget().data(); //makeWidgetでは右辺値が返ってくるので左辺値参照の.data()は使えない
}

image.png

項目 13:iterator よりも const_iterator を優先する

constを使うように、値を変更しないならconst_iteratorを使いましょう。
ワーニング関することはないのでスキップ。

項目 14:例外を発生させない関数は noexcept と宣言する

ワーニング関することはないのでスキップ。

項目 15:可能な場面では常に constexpr を用いる

ワーニング関することはないのでスキップ。

constexpr.cpp
constexpr int pow(int base, int exp) noexcept
{
    return (exp == 0 ? 1 : base * pow(base, exp - 1));//C++11は1行のみ
}

int main(void)
{
    constexpr int base = 2;
    base = 4;//constexprを変更してみる。ビルドエラー
    constexpr int exp = 3;
    std::array<int, pow(base, exp)> ary;
    std::cout << ary.size() << std::endl; // 8
}

念のためconstexprを変更してみたが、問題なくエラー
image.png

項目 16:const メンバ関数はスレッドセーフにする

constメンバ関数がmutableなメンバ変数の書き込みが競合する可能性があるケースでもワーニングはでない。
intなどの書き込みもワーニングはない。

constexpr.cpp

#include <iostream>
#include <vector>
#include <array>
#include <thread>
#include <mutex>
#include <vector>

std::mutex m;

void ThreadFuncInt(int thread,int* num) {
     m.lock();
    std::cout << "ThreadFuncInt " << thread << std::endl;
    *num *= 10;
    std::cout << *num << std::endl;
    m.unlock();

}



class Smple 
{
public:
    Smple(bool bFlag) :bCashed(bFlag) {};

    void Calc() const{
       // std::lock_guard<std::mutex> guard(m);
        if (!bCashed) 
        {
            std::cout << "bCashed False " << std::endl;
            bCashed = true;
        }
        else {
            std::cout << "bCashed true " << std::endl;
        }

        return;
    }

    void Calc2() const 
    {
        std::lock_guard<std::mutex> guard(m);
        if (!bCashed)
        {
            std::cout << "bCashed False " << std::endl;
            bCashed = true;
        }
        else {
            std::cout << "bCashed true " << std::endl;
        }
    }
private:
    mutable bool bCashed;
    mutable std::mutex m;
};

int main() {

    Smple p(false);


    int i = 10;
    std::vector<int> v(3,10);

    std::thread th2(ThreadFuncInt, 2, &i);//書き込み競合

    std::thread th4(ThreadFuncInt, 4, &i);//書き込み競合

    std::thread th5([&p]() {p.Calc(); });//書き込み競合の可能性
    std::thread th6([&p]() {p.Calc(); });//書き込み競合の可能性
    std::thread th7([&p]() {p.Calc2(); });


    th2.join();
    th4.join();
    th5.join();
    th6.join();
    th7.join();

    return 0;
}

項目 17:自動的に生成される特殊メンバ関数を理解する

visual studio上では、moveコンストラクタを自作した時に暗黙的に削除されたCopyコンストラクタを使用するとビルドエラーとなる。
また、自作したmoveコンストラクタでメンバ変数を全てmoveできていない場合はワーニングが表示されている。
Sider上でも、暗黙的に削除されたCopyコンストラクタを使用するとエラーとなる。
自作したmoveコンストラクタでメンバ変数を全てmoveできていない場合は何も教えてくれない。

move.cpp

class Widget1
{
public:
    Widget1(int a):m_a(a){}

    Widget1(Widget1&& original) noexcept
    {
       /// m_a = original.m_a;
    }

    void print() {
        std::cout << m_a << std::endl;
    }

private:
    int m_a;

};

class Widget2
{
public:
    Widget2(int a) :m_a(a) {}

    //テンプレートでムーブコンストラクタを自作した場合
    template<typename T>
    Widget2(const T&& original) noexcept
    {
        /// m_a = original.m_a;
    }

    void print() {
        std::cout << m_a << std::endl;
    }

private:
    int m_a;
};

int main() {

    Widget1 a(1);
    Widget1 b(std::move(a));
    b.print();//正常にmoveされていないので無効値が表示される
    Widget1 e = b;//ビルド時はE1776dでビルドエラー

    Widget2 c(1);
    Widget2 d(std::move(c));//テンプレートメンバ関数が特殊メンバ関数の生成を抑制することはない
    d.print();//1が表示される
    return 0;
}

image.png

続きは学習が進捗次第記載します

Siderで独自の文法チェックルールを作成する

これまで見てきた中で、Siderでは検出できないが、世の中的、あるいはプロジェクトの中では守るべきルールとというのが往々にしてある。
そのような場合にどのように対処するか記載する。

Siderがサポートしているツールの一つにGoodCheckというツールがある。
導入方法などはリンク先を参照していただきたい。

生成したgoodchek.ymlファイルに独自のルールを記述し、gooodcheck checkコマンドを実行すると指定したファイルを検査してくれる。
パターンマッチングしているようなので、プロジェクトが大きくなった時にどれほど時間がかかるかは分からない。
image.png

こんな感じで自分で定義したエラーを検知してくれる。
image.png

他にもRuby用とPHP用もある模様。
参考

0
1
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
1