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

Enable more checkers for clang-tidy in CI #5738

Merged
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 14 additions & 3 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
# maybe-* checks are only available on OneFlow custom clang-tidy and clangd
Checks: '-*, maybe-*'
# `maybe-*` checks are only available on OneFlow custom clang-tidy and clangd
# `-allow-enabling-analyzer-alpha-checkers` should be passed to clang-tidy for CSA checkers named `clang-analyzer-alpha.*` (or `-allow-enabling-alpha-checkers` for run-clang-tidy.py)
# `aggressive-binary-operation-simplification` should be enabled (via `-Xclang -analyzer-config -Xclang aggressive-binary-operation-simplification=true` in clang)
# there is some problem in `clang-analyzer-alpha.clone.*`, so do not enable it
# `clang-analyzer-alpha.deadcode.*` is just too verbose to enable
Checks: '-*, maybe-*, clang-analyzer-core.*, clang-analyzer-cplusplus.*, clang-analyzer-nullability.*, clang-analyzer-deadcode.*, clang-analyzer-security.*, clang-analyzer-optin.cplusplus.*, clang-analyzer-optin.performance.*, clang-analyzer-alpha.core.*, clang-analyzer-alpha.cplusplus.*, clang-analyzer-alpha.security.*, cppcoreguidelines-avoid-goto, cppcoreguidelines-init-variables, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-no-malloc, cppcoreguidelines-prefer-member-initializer, cppcoreguidelines-pro-type-member-init, cppcoreguidelines-pro-type-static-cast-downcast, cppcoreguidelines-slicing, cppcoreguidelines-special-member-functions, performance-unnecessary-value-param, performance-unnecessary-copy-initialization, performance-noexcept-move-constructor, performance-no-automatic-move, performance-move-const-arg, performance-implicit-conversion-in-loop, performance-for-range-copy, google-default-arguments, google-global-names-in-headers, google-explicit-constructor'
# TODO: treat all maybe warnings as errors when existing warnings are all fixed
WarningsAsErrors: 'maybe-unused'
WarningsAsErrors: 'maybe-unused, clang-analyzer-nullability.*, clang-analyzer-cplusplus.*, performance-implicit-conversion-in-loop, performance-move-const-arg, performance-no-automatic-move, performance-noexcept-move-constructor, google-default-arguments, google-global-names-in-headers'

CheckOptions:
# `cppcoreguidelines-special-member-functions` is enabled, refer to https://en.cppreference.com/w/cpp/language/rule_of_three
- key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor
value: True
- key: performance-move-const-arg.CheckTriviallyCopyableMove
value: False
2 changes: 1 addition & 1 deletion .github/workflows/simple.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ jobs:
-DBUILD_TESTING=ON \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON
cd ..
./run-clang-tidy.py -clang-tidy-binary ./clang-tidy-489012f-x86_64.AppImage -p build -quiet
./run-clang-tidy.py -clang-tidy-binary ./clang-tidy-489012f-x86_64.AppImage -p build -quiet -allow-enabling-alpha-checkers -extra-arg="-Xclang" -extra-arg="-analyzer-config" -extra-arg="-Xclang" -extra-arg="aggressive-binary-operation-simplification=true" '^((?!third_party_install).)+(?<!cfg.cpp)(?<!pb.cc)$'

hosted:
name: CPU-only
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -761,4 +761,4 @@ jobs:
-DBUILD_TESTING=ON \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON
cd ..
git diff -U0 ${{ github.event.pull_request.base.sha }} | ./clang-tidy-diff.py -clang-tidy-binary ./clang-tidy-489012f-x86_64.AppImage -path build -quiet -j $(nproc) -p1
git diff -U0 ${{ github.event.pull_request.base.sha }} | ./clang-tidy-diff.py -clang-tidy-binary ./clang-tidy-489012f-x86_64.AppImage -path build -quiet -j $(nproc) -p1 -extra-arg="-Xclang" -extra-arg="-analyzer-config" -extra-arg="-Xclang" -extra-arg="aggressive-binary-operation-simplification=true"
4 changes: 1 addition & 3 deletions oneflow/core/eager/eager_blob_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ Maybe<void> EagerBlobObject::InitBlob() {
{
header_buffer_.reset();
int64_t header_byte_size = blob_desc_.AlignedByteSizeOfBlobHeader();
const auto& FreeHeader = [header_byte_size](char* dptr) { std::free(dptr); };
char* ptr = reinterpret_cast<char*>(std::malloc(header_byte_size));
header_buffer_ = std::unique_ptr<char, std::function<void(char*)>>(ptr, FreeHeader);
header_buffer_ = std::make_unique<char[]>(header_byte_size);
}
blob_.reset(new Blob(*mem_case_, &blob_desc_, header_buffer_.get(), nullptr));
return Maybe<void>::Ok();
Expand Down
2 changes: 1 addition & 1 deletion oneflow/core/eager/eager_blob_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class EagerBlobObject final : public BlobObject {

private:
std::unique_ptr<Blob> blob_;
std::unique_ptr<char, std::function<void(char*)>> header_buffer_;
std::unique_ptr<char[]> header_buffer_;
std::shared_ptr<TensorBuffer> tensor_buffer_;
std::size_t blob_body_bytes_;
std::unique_ptr<MemoryAllocator> non_pod_initer_;
Expand Down
14 changes: 7 additions & 7 deletions oneflow/core/job_rewriter/quantization_aware_training.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,13 @@ Maybe<bool> InsertQuantOpAfterInt8Ops4QatConfig(const QatConfig& qat_config) {
user_op::UserOpConfWrapper MultiplyOp(const std::string& name, const std::string& x,
const std::string& y, const int64_t scope_symbol_id,
OpConfMap* inserted_ops) {
const auto op_wrapper = user_op::UserOpConfWrapperBuilder(name)
.Op("broadcast_mul")
.Input("x", x)
.Input("y", y)
.Output("z")
.ScopeSymbolId(scope_symbol_id)
.Build();
auto op_wrapper = user_op::UserOpConfWrapperBuilder(name)
.Op("broadcast_mul")
.Input("x", x)
.Input("y", y)
.Output("z")
.ScopeSymbolId(scope_symbol_id)
.Build();
(*inserted_ops)[name] = op_wrapper.op_conf();
return op_wrapper;
}
Expand Down
2 changes: 1 addition & 1 deletion oneflow/core/kernel/user_kernel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ class UserKernelComputeContext final : public user_op::KernelComputeContext {
const JobDesc& job_desc)
: user_op_conf_(kernel_conf.op_attribute().op_conf()),
device_ctx_(device_ctx),
base_ctx_(std::move(UserKernelBaseContext(kernel_conf, job_desc))) {
base_ctx_(kernel_conf, job_desc) {
auto InitInOrOut = [&](const PbMap<std::string, UserOpConf::ListString>& arg_map) {
for (const auto& it : arg_map) {
const std::string& arg_name = it.first;
Expand Down