[JIT] Fix clang-tidy warnings for jit/frontend (#47982)

Summary: Pull Request resolved: https://github.com/pytorch/pytorch/pull/47982

Test Plan: Imported from OSS

Reviewed By: ZolotukhinM

Differential Revision: D25258640

Pulled By: SplitInfinity

fbshipit-source-id: e2cf27130311904aa5b18e3232349604d01701a0

torch/csrc/jit/frontend/concrete_module_type.cpp

@@ -228,7 +228,7 @@ void ConcreteModuleTypeBuilder::addConstant(
         "\n:",
         match.reason());
   }
-  constants_.emplace(std::move(name), toIValue(value, match.type()));
+  constants_.emplace(std::move(name), toIValue(std::move(value), match.type()));
 }
 
 void ConcreteModuleTypeBuilder::addConstant(std::string name, IValue value) {
@@ -237,7 +237,7 @@ void ConcreteModuleTypeBuilder::addConstant(std::string name, IValue value) {
 
 void ConcreteModuleTypeBuilder::addAttribute(
     std::string name,
-    TypePtr type,
+    const TypePtr& type,
     bool isParameter,
     bool isBuffer) {
   TORCH_INTERNAL_ASSERT(type);
@@ -262,7 +262,7 @@ void ConcreteModuleTypeBuilder::addFunctionAttribute(
 
 void ConcreteModuleTypeBuilder::addBuiltinFunction(
     std::string name,
-    std::string symbol_name) {
+    const std::string& symbol_name) {
   builtinFunctions_.emplace(
       std::move(name), c10::Symbol::fromQualString(symbol_name));
 }

torch/csrc/jit/frontend/concrete_module_type.h

@@ -66,7 +66,7 @@ class VISIBILITY_HIDDEN ConcreteModuleTypeBuilder {
   void addConstant(std::string name, IValue value);
   void addAttribute(
       std::string name,
-      TypePtr type,
+      const TypePtr& type,
       bool isParameter,
       bool isBuffer);
   void addFunctionAttribute(
@@ -79,7 +79,7 @@ class VISIBILITY_HIDDEN ConcreteModuleTypeBuilder {
   void addOverload(
       std::string methodName,
       std::vector<std::string> overloadedMethodNames);
-  void addBuiltinFunction(std::string name, std::string symbol_name);
+  void addBuiltinFunction(std::string name, const std::string& symbol_name);
   void addFailedAttribute(std::string name, std::string failureReason);
   void addIgnoredAttribute(std::string name);
   void setIterableModuleKind(IterableModuleKind kind);
@@ -134,7 +134,7 @@ class VISIBILITY_HIDDEN ConcreteModuleTypeBuilder {
   };
 
  private:
-  ConcreteModuleTypeBuilder() {}
+  ConcreteModuleTypeBuilder() = default;
   ClassTypePtr createTypeFromThis() const;
 
   // If true, this type will never compare equally to anything else. This is
@@ -221,7 +221,7 @@ class VISIBILITY_HIDDEN ConcreteModuleType {
   void dump() const;
 
  private:
-  ConcreteModuleType() {}
+  ConcreteModuleType() = default;
 
   // The JIT type derived from this ConcreteModuleType.
   ConcreteModuleTypeBuilder data_;

torch/csrc/jit/frontend/exit_transforms.cpp

@@ -674,7 +674,7 @@ class DepthFirstGraphNodeIterator {
           // If either of the then or else blocks have nodes, the current block
           // and iterator position need to be saved on the stack to resume
           // processing later.
-          block_stack_.push_back({current_.first, current_.second});
+          block_stack_.emplace_back(current_.first, current_.second);
         }
 
         if (!then_block_empty && else_block_empty) {
@@ -690,7 +690,7 @@ class DepthFirstGraphNodeIterator {
         } else if (!then_block_empty && !else_block_empty) {
           // Set current_ to {then_block, then_block.begin()} and push the
           // else_block to the stack so that it will be processed after.
-          block_stack_.push_back({else_block, else_block->nodes().begin()});
+          block_stack_.emplace_back(else_block, else_block->nodes().begin());
           current_.first = then_block;
           current_.second = then_block->nodes().begin();
         }
@@ -704,7 +704,7 @@ class DepthFirstGraphNodeIterator {
           // If body_block is not empty, push the current block onto the stack
           // to resume processing it later and set current_ to {body_block,
           // body_block.begin()}.
-          block_stack_.push_back({current_.first, current_.second});
+          block_stack_.emplace_back(current_.first, current_.second);
 
           current_.first = body_block;
           current_.second = body_block->nodes().begin();

torch/csrc/jit/frontend/function_schema_parser.cpp

@@ -202,14 +202,14 @@ struct SchemaParser {
   IValue convertToList(
       TypeKind kind,
       const SourceRange& range,
-      std::vector<IValue> vs) {
+      const std::vector<IValue>& vs) {
     switch (kind) {
       case TypeKind::FloatType:
-        return fmap(vs, [](IValue v) { return v.toDouble(); });
+        return fmap(vs, [](const IValue& v) { return v.toDouble(); });
       case TypeKind::IntType:
-        return fmap(vs, [](IValue v) { return v.toInt(); });
+        return fmap(vs, [](const IValue& v) { return v.toInt(); });
       case TypeKind::BoolType:
-        return fmap(vs, [](IValue v) { return v.toBool(); });
+        return fmap(vs, [](const IValue& v) { return v.toBool(); });
       default:
         throw ErrorReport(range)
             << "lists are only supported for float or int types";
@@ -224,7 +224,7 @@ struct SchemaParser {
       } while (L.nextIf(','));
     }
     L.expect(']');
-    return convertToList(kind, tok.range, std::move(vs));
+    return convertToList(kind, tok.range, vs);
   }
 
   IValue parseTensorDefault(const SourceRange& range) {

torch/csrc/jit/frontend/ir_emitter.cpp

@@ -43,7 +43,7 @@ using ListAttributeMap = std::unordered_map<std::string, std::vector<Const>>;
 
 struct Refinement {
   Refinement(std::string identifier, TypePtr type)
-      : identifier_(std::move(identifier)), type_(type) {}
+      : identifier_(std::move(identifier)), type_(std::move(type)) {}
   const std::string& identifier() const {
     return identifier_;
   }
@@ -71,7 +71,7 @@ struct RefinementSet {
       : RefinementSet(
             Refinements({std::move(single_true)}),
             Refinements({std::move(single_false)})) {}
-  RefinementSet() {} // empty
+  RefinementSet() = default; // empty
   RefinementSet And(const RefinementSet& rhs) const {
     // if the result of an AND is true, both a & b had to be true,
     // so we take the union of a.true_refinements and b.true_refinements.
@@ -245,7 +245,7 @@ struct Environment {
     while (runner->next) {
       runner = runner->next.get();
     }
-    runner->error_messages[name] = msg;
+    runner->error_messages[name] = std::move(msg);
   }
 
   // see if type error has been set for a variable
@@ -281,7 +281,7 @@ struct Environment {
       TypePtr type) {
     auto g = b->owningGraph();
     g->insertNode(g->createStore(name, v))->setSourceRange(loc);
-    type_table[name] = type;
+    type_table[name] = std::move(type);
   }
 
   SugaredValuePtr findInThisFrame(const std::string& name) {
@@ -404,7 +404,7 @@ struct Environment {
             << " but is being assigned to a value of type "
             << as_simple_value->type()->repr_str();
       }
-      insertStore(name, loc, std::move(as_simple_value), annotated_type);
+      insertStore(name, loc, as_simple_value, annotated_type);
     } else {
       value_table[name] = std::move(value);
     }
@@ -1105,9 +1105,9 @@ struct to_ir {
   }
 
   RefinementSet findIsNoneRefinements(
-      Expr lhs,
+      const Expr& lhs,
       Value* lhs_value,
-      Expr rhs,
+      const Expr& rhs,
       Value* rhs_value,
       int tok) {
     if (rhs.kind() != TK_NONE && lhs.kind() == TK_NONE) {
@@ -1361,8 +1361,8 @@ struct to_ir {
   Value* emitIfExpr(
       const SourceRange& range,
       const CondValue& cond_value,
-      std::function<Value*()> true_expr,
-      std::function<Value*()> false_expr) {
+      const std::function<Value*()>& true_expr,
+      const std::function<Value*()>& false_expr) {
     Node* n = graph->insertNode(create(prim::If, range, 0));
     n->addInput(cond_value.value());
     auto* true_block = n->addBlock();
@@ -1371,7 +1371,7 @@ struct to_ir {
     auto emit_if_expr = [this, &range](
                             Block* b,
                             const RefinementSet& refinements,
-                            std::function<Value*()> expr_value) {
+                            const std::function<Value*()>& expr_value) {
       pushFrame(b);
       WithInsertPoint guard(b);
       insertRefinements(range, refinements);
@@ -1380,9 +1380,8 @@ struct to_ir {
       popFrame();
     };
 
-    emit_if_expr(true_block, cond_value.refinements(), std::move(true_expr));
-    emit_if_expr(
-        false_block, cond_value.refinements().Not(), std::move(false_expr));
+    emit_if_expr(true_block, cond_value.refinements(), true_expr);
+    emit_if_expr(false_block, cond_value.refinements().Not(), false_expr);
 
     auto true_type = true_block->outputs().at(0)->type();
     auto false_type = false_block->outputs().at(0)->type();
@@ -1602,7 +1601,7 @@ struct to_ir {
     // category checks: tuple_check = true, types = {float, int}
     struct GatheredTypes {
       GatheredTypes(ScriptTypeParser parser) : typeParser_(std::move(parser)) {}
-      void gather(Expr classinfo) {
+      void gather(const Expr& classinfo) {
         if (classinfo.kind() == TK_TUPLE_LITERAL) {
           for (Expr e : TupleLiteral(classinfo).inputs()) {
             gather(e);
@@ -1695,7 +1694,7 @@ struct to_ir {
   // semantics specified at
   // https://github.com/onnx/onnx/blob/master/docs/Operators.md#Loop
   void emitLoopCommon(
-      SourceRange range,
+      const SourceRange& range,
       const std::function<void()>& emit_body,
       const SugaredValuePtr& iter_val,
       c10::optional<List<Expr>> targets,
@@ -1760,7 +1759,7 @@ struct to_ir {
   void emitUnrolledLoop(
       const SourceRange& loc,
       const std::function<void()>& emit_body,
-      SugaredValuePtr iterable,
+      const SugaredValuePtr& iterable,
       const List<Expr>& targets) {
     auto static_len = iterable->staticLen();
     TORCH_INTERNAL_ASSERT(
@@ -3073,8 +3072,8 @@ struct to_ir {
     // through RPC in TorchScript,
     // Ideally, function value in JIT IR is first-class citizen and
     // The RPC C++ entry API can take c10::Function directly.
-    auto rpcMinInputs = 2;
-    auto rpcMaxInputs = 5; // NOLINT
+    size_t rpcMinInputs = 2;
+    size_t rpcMaxInputs = 5; // NOLINT
     std::string op_name = rpc_op.toUnqualString();
     if (apply.inputs().size() < rpcMinInputs ||
         apply.inputs().size() > rpcMaxInputs) {

torch/csrc/jit/frontend/lexer.h

@@ -377,8 +377,8 @@ struct Token {
 };
 
 struct Lexer {
-  explicit Lexer(const std::shared_ptr<Source>& source)
-      : source(source),
+  explicit Lexer(std::shared_ptr<Source> source)
+      : source(std::move(source)),
         pos(0),
         nesting(0),
         indent_stack(),

torch/csrc/jit/frontend/parse_string_literal.h

@@ -12,10 +12,6 @@ inline bool isCharCount(char c, const std::string& str, size_t start, int len) {
       std::count(str.begin() + start, str.begin() + start + len, c) == len;
 }
 
-inline static bool isOctal(char c) {
-  return c >= '0' && c < '8';
-}
-
 inline c10::optional<char> parseOctal(const std::string& str, size_t pos) {
   //\xxx where x are 0-7
   if (pos + 3 >= str.size())

torch/csrc/jit/frontend/parser.cpp

@@ -163,14 +163,11 @@ struct ParserImpl {
       case TK_STRINGLITERAL: {
         prefix = parseConcatenatedStringLiterals();
       } break;
+      case TK_ELLIPSIS:
       case TK_DOTS: {
         prefix = Dots::create(L.cur().range);
         L.next();
       } break;
-      case TK_ELLIPSIS: {
-        prefix = Dots::create(L.cur().range);
-        L.next();
-      } break;
       default: {
         Ident name = parseIdent();
         prefix = Var::create(name.range(), name);

torch/csrc/jit/frontend/resolver.h

@@ -25,7 +25,7 @@ using ResolverPtr = std::shared_ptr<Resolver>;
  * handle the method.
  */
 struct Resolver {
-  virtual ~Resolver() {}
+  virtual ~Resolver() = default;
 
   // Resolve a given name to a SugaredValue. This takes the method `m` that the
   // caller is currently constructing, since we may need to insert nodes into

torch/csrc/jit/frontend/schema_matching.cpp

@@ -519,7 +519,7 @@ std::pair<size_t, MatchedSchema> matchSchemas(
           render_errors ? &failure_messages : nullptr,
           allow_conversions);
       if (matched_schema) {
-        return std::make_pair(i, std::move(*matched_schema));
+        return std::make_pair(i, *matched_schema);
       }
     }
   }
@@ -551,8 +551,8 @@ static Value* packOutputs(
   TupleTypePtr named_tuple = nullptr;
   if (field_names) {
     auto types = fmap(values, [](Value* v) { return v->type(); });
-    named_tuple = TupleType::createNamed(
-        c10::nullopt, field_names.value(), std::move(types));
+    named_tuple =
+        TupleType::createNamed(c10::nullopt, field_names.value(), types);
   }
   return g.insertNode(g.createTuple(values, named_tuple))->output();
 }
@@ -592,6 +592,7 @@ Value* emitBuiltinCall(
 
   std::stringstream failure_messages;
   std::vector<const FunctionSchema*> schemas;
+  schemas.reserve(variants.size());
   for (const std::shared_ptr<Operator>& op : variants) {
     schemas.push_back(&op->schema());
   }

torch/csrc/jit/frontend/script_type_parser.h

@@ -15,7 +15,7 @@ namespace jit {
  */
 class TORCH_API ScriptTypeParser {
  public:
-  explicit ScriptTypeParser() {}
+  explicit ScriptTypeParser() = default;
   explicit ScriptTypeParser(ResolverPtr resolver)
       : resolver_(std::move(resolver)) {}
 

torch/csrc/jit/frontend/sugared_value.cpp

@@ -560,7 +560,7 @@ SugaredValuePtr IterableTree::getitem(
 void IterableTree::addChild(
     const SourceRange& range,
     Function& m,
-    const SugaredValuePtr iter_value) {
+    const SugaredValuePtr& iter_value) {
   c10::optional<int64_t> child_len = iter_value->staticLen();
   if (children_.size() == 0) {
     unroll_length_ = child_len;

torch/csrc/jit/frontend/sugared_value.h

@@ -2,6 +2,7 @@
 #include <functional>
 #include <memory>
 #include <string>
+#include <utility>
 
 #include <ATen/core/interned_strings.h>
 #include <torch/csrc/jit/api/module.h>
@@ -233,7 +234,7 @@ struct TORCH_API BuiltinFunction : public SugaredValue {
 
 struct TORCH_API SugaredTupleValue : public SugaredValue {
   explicit SugaredTupleValue(std::vector<std::shared_ptr<SugaredValue>> tup)
-      : tup_(tup){};
+      : tup_(std::move(tup)){};
 
   std::vector<std::shared_ptr<SugaredValue>> asTuple(
       const SourceRange& loc,
@@ -296,7 +297,7 @@ struct TORCH_API SugaredTupleValue : public SugaredValue {
 
 struct TORCH_API BuiltinModule : public SugaredValue {
   BuiltinModule(std::string name, c10::optional<int64_t> version = at::nullopt)
-      : name(std::move(name)), version(std::move(version)) {}
+      : name(std::move(name)), version(version) {}
 
   std::string kind() const override {
     return "builtin module";
@@ -373,7 +374,7 @@ struct TORCH_API NamedTupleConstructor : public SugaredValue {
 };
 
 struct FunctionValue : public SugaredValue {
-  FunctionValue(Function* callee) : callees_({std::move(callee)}) {}
+  FunctionValue(Function* callee) : callees_({callee}) {}
   FunctionValue(const StrongFunctionPtr& p)
       : callees_({p.function_}), cu_(p.cu_) {}
   FunctionValue(const std::vector<StrongFunctionPtr>& callees) {
@@ -438,9 +439,9 @@ struct TORCH_API ClosureValue : public SugaredValue {
 // defines how a method obtained from a module/class/interface behaves in script
 struct MethodValue : public SugaredValue {
   MethodValue(Value* self, std::vector<std::string> method_names)
-      : self_(std::move(self)), method_names_(std::move(method_names)) {}
+      : self_(self), method_names_(std::move(method_names)) {}
   MethodValue(Value* self, std::string method_name)
-      : MethodValue(self, std::vector<std::string>({method_name})) {}
+      : MethodValue(self, std::vector<std::string>({std::move(method_name)})) {}
 
   std::string kind() const override {
     return "method";
@@ -670,7 +671,7 @@ struct TORCH_API IterableTree : SugaredValue {
   void addChild(
       const SourceRange& range,
       Function& m,
-      const SugaredValuePtr iter_value);
+      const SugaredValuePtr& iter_value);
 
   std::vector<SugaredValuePtr> get_children() {
     return children_;
@@ -738,7 +739,7 @@ struct TORCH_API ExceptionMessageValue : public SugaredValue {
 };
 
 struct TORCH_API ExceptionValue : public SugaredValue {
-  explicit ExceptionValue(const std::string& message) : message_(message) {}
+  explicit ExceptionValue(std::string message) : message_(std::move(message)) {}
 
   std::string kind() const override {
     return "exception";

torch/csrc/jit/frontend/tree_views.h

@@ -226,8 +226,9 @@ struct Ident : public TreeView {
   const std::string& name() const {
     return subtree(0)->stringValue();
   }
-  static Ident create(const SourceRange& range, const std::string& name) {
-    return Ident(Compound::create(TK_IDENT, range, {String::create(name)}));
+  static Ident create(const SourceRange& range, std::string name) {
+    return Ident(
+        Compound::create(TK_IDENT, range, {String::create(std::move(name))}));
   }
 };
 
@@ -403,7 +404,7 @@ struct Def : public TreeView {
     auto new_ident = Ident::create(name().range(), std::move(new_name));
     return create(range(), new_ident, decl(), statements());
   }
-  Def withDecl(Decl decl) const {
+  Def withDecl(const Decl& decl) const {
     return create(range(), name(), decl, statements());
   }
   Ident name() const {
@@ -839,7 +840,7 @@ struct Const : public Expr {
   }
   int64_t asIntegral() const {
     try {
-      return c10::stoll(subtree(0)->stringValue(), /*pos=*/0, /*base=*/0);
+      return c10::stoll(subtree(0)->stringValue(), /*__idx=*/0, /*base=*/0);
     } catch (const std::out_of_range& e) {
       throw ErrorReport(range()) << "Integral constant out of range "
                                     "(must fit in a signed 64 bit integer)";