As discussed in the https://reviews.llvm.org/D148697 review.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Overall good, my main concern is base class name & location.
clang-tools-extra/clang-tidy/performance/NoexceptDestructorCheck.h | ||
---|---|---|
23 | Consider putting "Base" into a name, so that it woudn't be considered a full check, and it can be in this module, no need to put it into utils | |
24–26 | we got c++17, so delegate constructor. | |
27–28 | consider moving this into base class | |
clang-tools-extra/clang-tidy/utils/NoexceptFunctionCheck.h | ||
34–38 ↗ | (On Diff #532388) | you can move those into protected. |
The is compiler error in CI "Cannot find source file: BaseNoexceptFunctionCheck.cpp".
Consider naming it NoexceptFunctionBase.cpp or NoexceptFunctionBaseCheck.cpp unless there are already other .cpp files in some other modules that start with Base, in such case its fine.
Main reason for this is to somehow show others that this file is not an full check, but more a base for some checks.
clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.h | ||
---|---|---|
31 | you can put those all as private here in derived class. |
So somehow I didn't add the actual BaseNoexceptFunctionCheck files, thats why the build failed.
Also implemented the two new suggestions.
If there are no more problems, I would kindly ask for someone to push this no my behalf :)
Consider putting "Base" into a name, so that it woudn't be considered a full check, and it can be in this module, no need to put it into utils