Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CNativeWとCNativeAの自前コンストラクタ実装を削除する #1516

Merged

Conversation

berryzplus
Copy link
Contributor

@berryzplus berryzplus commented Jan 24, 2021

PR の目的

SonarCloud解析でC++関数の誤用が指摘されているので、CNativeWとCNativeAの自前コンストラクタ実装を削除します。

カテゴリ

  • リファクタリング

PR の背景

#1477 で対処しようとしていた問題を解決します。

SonarCloudの指摘は std::forward の使い方が誤っているなんですが、
std::forwardstd::move に置き換えても警告が残ることが分かっています。

どんな警告が残るかというと、
管理すべきリソースを持たないクラスがmove semantics実装すんなヴォケ!
というものです。

ここで言う「リソース」にはメモリポインタを含みません。メモリポインタ自体は単なるアドレス値であり、プロセス空間内の特定の位置を指し示す数値です。同じメモリでもメモリハンドルなら「リソース」に該当するのでややこしいです。CNativeWとCNativeAはメモリアドレスしか扱わないクラスなのでC++的には「リソース」を持たないクラスに当たるらしいです。

PR のメリット

SonarCloudのBugs指摘を数件解消できます。

PR のデメリット (トレードオフとかあれば)

とくに思い当たらないです。

仕様・動作説明

アプリの仕様・動作とは関係ない変更です。

実際にメモリ管理を行っているのはCMemoryなので、CNativeWとCNativeAの独自定義コンストラクタを削っても影響はないと考えられます。

PR の影響範囲

実際の影響は発生しないと考えられます。

テスト内容

追加のテストが必要かどうか、PRを投げてから判断します。
 👇
既にある単体テストのみで十分と判断したので追加はなしです。

関連 issue, PR

#1477

参考資料

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

@berryzplus berryzplus marked this pull request as ready for review January 24, 2021 10:53
Copy link
Member

@kengoide kengoide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

問題ないものと思います。

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。
マージしちゃいます。

@berryzplus berryzplus merged commit 2acbbb3 into sakura-editor:master Jan 24, 2021
@berryzplus berryzplus deleted the feature/remove_meanless_ctor branch January 24, 2021 12:52
@beru beru added the refactoring リファクタリング 【ChangeLog除外】 label Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring リファクタリング 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants