From da9c94da333cf3190c6f9f647139cc567a723f81 Mon Sep 17 00:00:00 2001 From: Gabor Buella Date: Wed, 13 Feb 2019 02:41:42 +0100 Subject: [PATCH] Clang build fixes (#15628) * Remove some superfluous std::move calls The std:move triggered a build error (with -Werror): ``` [ 9%] Building CXX object paddle/fluid/memory/allocation/CMakeFiles/allocator_facade.dir/allocator_facade.cc.o /home/tej/code/gbuella_paddle/paddle/fluid/memory/allocation/allocator_facade.cc:86:29: error: moving a temporary object prevents copy elision [-Werror,-Wpessimizing-move] [this] { return std::move(CreateAllocatorWithChunk()); }, capacity); ^ /home/tej/code/gbuella_paddle/paddle/fluid/memory/allocation/allocator_facade.cc:86:29: note: remove std::move call here [this] { return std::move(CreateAllocatorWithChunk()); }, capacity); ^~~~~~~~~~ ~ 1 error generated. ``` See: https://reviews.llvm.org/D7633 * Remove a superfluous lambda capture from framework/operator.h ``` [ 10%] Building CXX object paddle/fluid/platform/CMakeFiles/device_context.dir/init.cc.o In file included from /home/tej/code/gbuella_paddle/paddle/fluid/platform/init.cc:19: /home/tej/code/gbuella_paddle/paddle/fluid/framework/operator.h:229:21: error: lambda capture 'this' is not used [-Werror,-Wunused-lambda-capture] [this](Variable* var) { return var; }); ^~~~ 1 error generated. ``` Changing it to `return it->second;`, as is in the function below. * Rethrow an exception (instead of copying it) ``` [ 11%] Building CXX object paddle/fluid/framework/CMakeFiles/operator.dir/operator.cc.o /home/tej/code/gbuella_paddle/paddle/fluid/framework/operator.cc:191:13: error: local variable 'exception' will be copied despite being thrown by name [-Werror,-Wreturn-std-move] throw exception; ^~~~~~~~~ /home/tej/code/gbuella_paddle/paddle/fluid/framework/operator.cc:191:13: note: call 'std::move' explicitly to avoid copying throw exception; ^~~~~~~~~ std::move(exception) ``` See https://reviews.llvm.org/D43322 for an explanation of this diagnostic message. * Remove an unused variable ``` /home/tej/code/gbuella_paddle/paddle/fluid/framework/operator.cc:884:16: error: private field 'scope_' is not used [-Werror,-Wunused-private-field] const Scope& scope_; ^ ``` * struct ComputationOpHandle -> class ComputationOpHandle ``` [ 13%] Building CXX object paddle/fluid/framework/details/CMakeFiles/memory_early_delete_pass.dir/memory_early_delete_pass.cc.o In file included from /home/tej/code/gbuella_paddle/paddle/fluid/framework/details/memory_early_delete_pass.cc:21: /home/tej/code/gbuella_paddle/paddle/fluid/framework/details/reference_count_pass_helper.h:30:1: error: class 'ComputationOpHandle' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Werror,-Wmismatched-tags] class ComputationOpHandle; ^ /home/tej/code/gbuella_paddle/paddle/fluid/framework/details/computation_op_handle.h:29:8: note: previous use is here struct ComputationOpHandle : public OpHandleBase { ^ /home/tej/code/gbuella_paddle/paddle/fluid/framework/details/reference_count_pass_helper.h:30:1: note: did you mean struct here? class ComputationOpHandle; ^~~~~ struct 1 error generated. ``` * Fix name() methods under fluid/operators ``` In file included from /home/tej/code/gbuella_paddle/paddle/fluid/operators/jit/gen/act.cc:15: In file included from /home/tej/code/gbuella_paddle/paddle/fluid/operators/jit/gen/act.h:19: /home/tej/code/gbuella_paddle/paddle/fluid/operators/jit/gen/jitcode.h:71:23: error: 'name' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override] virtual const char* name() const = 0; ^ /home/tej/code/gbuella_paddle/paddle/fluid/operators/jit/gen_base.h:31:23: note: overridden virtual function is here virtual const char* name() const = 0; ^ ``` test=develop --- paddle/fluid/framework/details/computation_op_handle.h | 2 +- .../framework/details/parallel_ssa_graph_executor.cc | 4 ++-- paddle/fluid/framework/ir/graph.cc | 2 +- paddle/fluid/framework/operator.cc | 9 ++++----- paddle/fluid/framework/operator.h | 7 +------ paddle/fluid/inference/analysis/ir_pass_manager.cc | 2 +- paddle/fluid/inference/api/analysis_predictor.cc | 2 +- paddle/fluid/memory/allocation/allocator_facade.cc | 2 +- paddle/fluid/operators/jit/gen/act.h | 1 - paddle/fluid/operators/jit/gen/blas.h | 2 +- paddle/fluid/operators/jit/gen/hopv.h | 2 +- paddle/fluid/operators/jit/gen/jitcode.h | 1 - paddle/fluid/operators/jit/gen/matmul.h | 2 +- paddle/fluid/operators/jit/gen/seqpool.h | 2 +- paddle/fluid/pybind/inference_api.cc | 4 ++-- 15 files changed, 18 insertions(+), 26 deletions(-) diff --git a/paddle/fluid/framework/details/computation_op_handle.h b/paddle/fluid/framework/details/computation_op_handle.h index 601ae4f8c6d..1e3dbb1e44e 100644 --- a/paddle/fluid/framework/details/computation_op_handle.h +++ b/paddle/fluid/framework/details/computation_op_handle.h @@ -26,7 +26,7 @@ namespace paddle { namespace framework { namespace details { -struct ComputationOpHandle : public OpHandleBase { +class ComputationOpHandle : public OpHandleBase { public: ComputationOpHandle(ir::Node *node, Scope *scope, platform::Place place, size_t scope_idx); diff --git a/paddle/fluid/framework/details/parallel_ssa_graph_executor.cc b/paddle/fluid/framework/details/parallel_ssa_graph_executor.cc index 128aaa33a2c..e8deb5bfc6c 100644 --- a/paddle/fluid/framework/details/parallel_ssa_graph_executor.cc +++ b/paddle/fluid/framework/details/parallel_ssa_graph_executor.cc @@ -65,7 +65,7 @@ FeedFetchList ParallelSSAGraphExecutor::Run( if (pool_) { run_futures.emplace_back(pool_->enqueue(std::move(call))); } else { - fetch_data.emplace_back(std::move(call())); + fetch_data.emplace_back(call()); } } @@ -74,7 +74,7 @@ FeedFetchList ParallelSSAGraphExecutor::Run( if (exception_holder_.IsCaught()) { f.wait(); } else { - fetch_data.emplace_back(std::move(f.get())); + fetch_data.emplace_back(f.get()); } } } diff --git a/paddle/fluid/framework/ir/graph.cc b/paddle/fluid/framework/ir/graph.cc index 3eb5bdba3b7..4b5c846f327 100644 --- a/paddle/fluid/framework/ir/graph.cc +++ b/paddle/fluid/framework/ir/graph.cc @@ -76,7 +76,7 @@ std::map> Graph::InitFromProgram( var->inputs.push_back(node); } } - return std::move(var_nodes); + return var_nodes; } void Graph::ResolveHazard( diff --git a/paddle/fluid/framework/operator.cc b/paddle/fluid/framework/operator.cc index 9d6c10ab9e3..b22523e0f42 100644 --- a/paddle/fluid/framework/operator.cc +++ b/paddle/fluid/framework/operator.cc @@ -188,14 +188,14 @@ void OperatorBase::Run(const Scope& scope, const platform::Place& place) { VLOG(3) << place << " " << DebugStringEx(&scope); } catch (platform::EnforceNotMet exception) { if (Attrs().count("sub_block") != 0) { - throw exception; + throw; } auto& callstack = Attr>( OpProtoAndCheckerMaker::OpCreationCallstackAttrName()); if (callstack.empty()) { - throw exception; + throw; } std::ostringstream sout; sout << "Invoke operator " << Type() << " error.\n"; @@ -206,7 +206,7 @@ void OperatorBase::Run(const Scope& scope, const platform::Place& place) { sout << "C++ Callstacks: \n"; sout << exception.err_str_; exception.err_str_ = sout.str(); - throw exception; + throw; } catch (...) { std::rethrow_exception(std::current_exception()); } @@ -589,7 +589,7 @@ class RuntimeInferShapeContext : public InferShapeContext { public: RuntimeInferShapeContext(const OperatorBase& op, const Scope& scope, const RuntimeContext& ctx) - : op_(op), scope_(scope), ctx_(ctx) {} + : op_(op), ctx_(ctx) {} bool HasInput(const std::string& name) const override { // has only one input @@ -881,7 +881,6 @@ class RuntimeInferShapeContext : public InferShapeContext { } const OperatorBase& op_; - const Scope& scope_; const RuntimeContext& ctx_; }; diff --git a/paddle/fluid/framework/operator.h b/paddle/fluid/framework/operator.h index 40d935a5ff9..e33214b44bb 100644 --- a/paddle/fluid/framework/operator.h +++ b/paddle/fluid/framework/operator.h @@ -222,12 +222,7 @@ class ExecutionContext { if (it == ctx_.inputs.end()) { return {}; } - std::vector res; - res.reserve(it->second.size()); - std::transform(it->second.begin(), it->second.end(), - std::back_inserter(res), - [this](Variable* var) { return var; }); - return res; + return {it->second.begin(), it->second.end()}; } std::vector MultiOutputVar(const std::string& name) const { diff --git a/paddle/fluid/inference/analysis/ir_pass_manager.cc b/paddle/fluid/inference/analysis/ir_pass_manager.cc index 7476c199cfd..8d5ee36ae62 100644 --- a/paddle/fluid/inference/analysis/ir_pass_manager.cc +++ b/paddle/fluid/inference/analysis/ir_pass_manager.cc @@ -101,7 +101,7 @@ std::unique_ptr IRPassManager::Apply(std::unique_ptr graph) { } graph = pass->Apply(std::move(graph)); } - return std::move(graph); + return graph; } framework::proto::ProgramDesc IRPassManager::AcquireProgram( diff --git a/paddle/fluid/inference/api/analysis_predictor.cc b/paddle/fluid/inference/api/analysis_predictor.cc index da2e9803f04..712e010db43 100644 --- a/paddle/fluid/inference/api/analysis_predictor.cc +++ b/paddle/fluid/inference/api/analysis_predictor.cc @@ -421,7 +421,7 @@ std::unique_ptr CreatePaddlePredictor< if (!dynamic_cast(predictor.get())->Init(nullptr)) { return nullptr; } - return std::move(predictor); + return predictor; } void AnalysisPredictor::PrepareFeedFetch() { diff --git a/paddle/fluid/memory/allocation/allocator_facade.cc b/paddle/fluid/memory/allocation/allocator_facade.cc index 794d729bdc1..ea0b729dc6f 100644 --- a/paddle/fluid/memory/allocation/allocator_facade.cc +++ b/paddle/fluid/memory/allocation/allocator_facade.cc @@ -83,7 +83,7 @@ class ChunkedAllocator : public Allocator { VLOG(1) << "Create AutoIncrementAllocator with chunk_size " << max_chunk_size_ << " and capacity " << capacity; default_allocator_ = std::make_shared( - [this] { return std::move(CreateAllocatorWithChunk()); }, capacity); + [this] { return CreateAllocatorWithChunk(); }, capacity); } } diff --git a/paddle/fluid/operators/jit/gen/act.h b/paddle/fluid/operators/jit/gen/act.h index 68e66f9298c..1664dfa906b 100644 --- a/paddle/fluid/operators/jit/gen/act.h +++ b/paddle/fluid/operators/jit/gen/act.h @@ -63,7 +63,6 @@ class VActFunc : public JitCode { public: explicit VActFunc(size_t code_size, void* code_ptr) : JitCode(code_size, code_ptr) {} - virtual const char* name() const = 0; virtual void genCode() = 0; protected: diff --git a/paddle/fluid/operators/jit/gen/blas.h b/paddle/fluid/operators/jit/gen/blas.h index 66a97c1be50..e9911392666 100644 --- a/paddle/fluid/operators/jit/gen/blas.h +++ b/paddle/fluid/operators/jit/gen/blas.h @@ -41,7 +41,7 @@ class VXXJitCode : public JitCode { this->genCode(); } - virtual const char* name() const { + virtual const char* name() const override { std::string base = "VXXJitCode"; if (scalar_index_ == 1) { base += "_Scalar"; diff --git a/paddle/fluid/operators/jit/gen/hopv.h b/paddle/fluid/operators/jit/gen/hopv.h index d3bc94b63d3..c336fe73fe5 100644 --- a/paddle/fluid/operators/jit/gen/hopv.h +++ b/paddle/fluid/operators/jit/gen/hopv.h @@ -35,7 +35,7 @@ class HOPVJitCode : public JitCode { this->genCode(); } - virtual const char* name() const { + virtual const char* name() const override { std::string base = "VXXJitCode"; if (type_ == operand_type::MAX) { base += "_MAX"; diff --git a/paddle/fluid/operators/jit/gen/jitcode.h b/paddle/fluid/operators/jit/gen/jitcode.h index c388109604b..91058f6cf66 100644 --- a/paddle/fluid/operators/jit/gen/jitcode.h +++ b/paddle/fluid/operators/jit/gen/jitcode.h @@ -68,7 +68,6 @@ class JitCode : public GenBase, public Xbyak::CodeGenerator { (code_size % 4096 != 0 ? (code_size / 4096 + 1) * 4096 : code_size), code_ptr) {} - virtual const char* name() const = 0; virtual void genCode() = 0; size_t getSize() const override { return CodeGenerator::getSize(); } diff --git a/paddle/fluid/operators/jit/gen/matmul.h b/paddle/fluid/operators/jit/gen/matmul.h index 626baa8f738..7976e3112da 100644 --- a/paddle/fluid/operators/jit/gen/matmul.h +++ b/paddle/fluid/operators/jit/gen/matmul.h @@ -36,7 +36,7 @@ class MatMulJitCode : public JitCode { this->genCode(); } - virtual const char* name() const { + virtual const char* name() const override { std::string base = "MatMulJitCode"; base = base + "_M" + std::to_string(m_) + "_N" + std::to_string(n_) + "_K" + std::to_string(k_); diff --git a/paddle/fluid/operators/jit/gen/seqpool.h b/paddle/fluid/operators/jit/gen/seqpool.h index fcbbb3c84c5..c464c2eac85 100644 --- a/paddle/fluid/operators/jit/gen/seqpool.h +++ b/paddle/fluid/operators/jit/gen/seqpool.h @@ -38,7 +38,7 @@ class SeqPoolJitCode : public JitCode { this->genCode(); } - virtual const char* name() const { + virtual const char* name() const override { std::string base = "SeqPoolJitCode"; if (type_ == SeqPoolType::kSum) { base += "_Sum"; diff --git a/paddle/fluid/pybind/inference_api.cc b/paddle/fluid/pybind/inference_api.cc index 39e47be606c..7db2bb451b4 100644 --- a/paddle/fluid/pybind/inference_api.cc +++ b/paddle/fluid/pybind/inference_api.cc @@ -74,12 +74,12 @@ void BindPaddleBuf(py::module *m) { .def(py::init([](std::vector &data) { auto buf = PaddleBuf(data.size() * sizeof(float)); std::memcpy(buf.data(), static_cast(data.data()), buf.length()); - return std::move(buf); + return buf; })) .def(py::init([](std::vector &data) { auto buf = PaddleBuf(data.size() * sizeof(int64_t)); std::memcpy(buf.data(), static_cast(data.data()), buf.length()); - return std::move(buf); + return buf; })) .def("resize", &PaddleBuf::Resize) .def("reset", -- GitLab