[5/N] Fix clang-tidy warnings in jit (#131969)

Follows #131903
Pull Request resolved: https://github.com/pytorch/pytorch/pull/131969
Approved by: https://github.com/ezyang
This commit is contained in:
cyy
2024-07-27 17:54:18 +00:00
committed by PyTorch MergeBot
parent 918ece4f4d
commit 8e5a367311
10 changed files with 82 additions and 94 deletions

View File

@ -7,8 +7,7 @@
#include <string>
#include <vector>
namespace torch {
namespace jit {
namespace torch::jit {
enum class IterableModuleKind { NONE, LIST, DICT, PARAMLIST, PARAMDICT };
class ConcreteModuleType;
@ -237,5 +236,4 @@ class VISIBILITY_HIDDEN ConcreteModuleType {
TypePtr jitType_;
};
} // namespace jit
} // namespace torch
} // namespace torch::jit

View File

@ -289,8 +289,7 @@ struct ExitTransformer {
else_pair = ExitPair(else_pair.hasExited(), exit_vals);
}
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
Value* has_exited;
Value* has_exited = nullptr;
if (if_status == ExitStatus::WILL) {
// Need to maintain the invariant that if hasExited() == true_val_
// then we have exited.
@ -551,10 +550,8 @@ static bool inlineConsecutiveIfs(Node* node) {
bool else_value = maybe_else_value->toBool();
for (const auto i : c10::irange(2)) {
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
Block* first_if_block;
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
Block* second_if_block;
Block* first_if_block = nullptr;
Block* second_if_block = nullptr;
if (i == 0) {
first_if_block = first_if.thenBlock();

View File

@ -2364,8 +2364,7 @@ struct to_ir {
{
Block* condition_block = n->addBlock();
pushFrame(condition_block);
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
Value* out;
Value* out = nullptr;
if (cond) {
WithInsertPoint insert(condition_block);
out = emitToBool(cond.value().range(), emitExpr(cond.value()));
@ -2846,10 +2845,7 @@ struct to_ir {
if (sliceable->type()->isSubtypeOf(*TensorType::get())) {
// If it's a tensor, just fully evaluate the subscript operation and emit
// an in-place assignment
std::vector<Value*> tensorIndices;
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
Value* sliced;
std::tie(sliced, tensorIndices) = emitIntAndSliceIndexing(
auto [sliced, tensorIndices] = emitIntAndSliceIndexing(
lhs.range(), sliceable, lhs.subscript_exprs());
const auto slicedArg = NamedValue(stmt.lhs().range(), "self", sliced);
@ -4201,12 +4197,10 @@ struct to_ir {
at::ArrayRef<NamedValue> args,
at::ArrayRef<NamedValue> kwargs) {
auto g = method.graph();
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
Node* fork_node;
TypePtr out_type;
fork_node = g->insertNode(method.graph()->create(prim::forkClosure, 1))
->setSourceRange(loc);
auto fork_node = g->insertNode(method.graph()->create(prim::forkClosure, 1))
->setSourceRange(loc);
// We create a fork by emitting a closure and setting the closure output
// into the fork input. If a closure doesn't already exist, we create one.
@ -4240,8 +4234,7 @@ struct to_ir {
at::ArrayRef<NamedValue> args,
at::ArrayRef<NamedValue> kwargs) {
auto g = method.graph();
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
TypePtr out_type;
TypePtr out_type{};
auto await_node =
g->insertNode(method.graph()->create(prim::awaitableClosure, 1))
@ -4277,7 +4270,7 @@ struct to_ir {
// Ideally, function value in JIT IR is first-class citizen and
// The RPC C++ entry API can take c10::Function directly.
size_t rpcMinInputs = 2;
size_t rpcMaxInputs = 5; // NOLINT
size_t rpcMaxInputs = 5;
std::string op_name = rpc_op.toUnqualString();
if (apply.inputs().size() < rpcMinInputs ||
apply.inputs().size() > rpcMaxInputs) {

View File

@ -2,7 +2,6 @@
#include <c10/util/Exception.h>
#include <mutex>
#include <string>
#include <unordered_map>

View File

@ -13,11 +13,6 @@
#include <string>
#include <vector>
C10_CLANG_DIAGNOSTIC_PUSH()
#if C10_CLANG_HAS_WARNING("-Wshorten-64-to-32")
C10_CLANG_DIAGNOSTIC_IGNORE("-Wshorten-64-to-32")
#endif
namespace torch::jit {
// single character tokens are just the character itself '+'
@ -142,7 +137,7 @@ TORCH_API int stringToKind(const std::string& str);
struct TokenTrie;
using TokenTrieRef = std::unique_ptr<TokenTrie>;
struct TokenTrie {
TokenTrie() {}
TokenTrie() = default;
void insert(const char* str, int tok) {
if (*str == '\0') {
AT_ASSERT(kind == 0);
@ -171,7 +166,6 @@ struct TokenTrie {
// once.
struct TORCH_API SharedParserData {
SharedParserData() : head(new TokenTrie()) {
std::stringstream ss;
for (const char* c = valid_single_char_tokens; *c; c++) {
std::string str(1, *c);
head->insert(str.c_str(), *c);
@ -321,8 +315,7 @@ struct TORCH_API SharedParserData {
if (first == '-' || first == '+' || isalpha(first))
return false;
const char* startptr = str.data() + start;
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
char* endptr;
char* endptr = nullptr;
torch::jit::strtod_c(startptr, &endptr);
*len = endptr - startptr;
// check if the number is complex valued
@ -495,9 +488,8 @@ struct Lexer {
break;
case TK_WHITESPACE:
case TK_WHITESPACE_EOF: {
const auto depth = static_cast<int64_t>(
r.kind == TK_WHITESPACE_EOF ? indent_stack.front()
: r.range.size());
const auto depth =
r.kind == TK_WHITESPACE_EOF ? indent_stack.front() : r.range.size();
// note: TK_WHITESPACE_EOF is whitespace right before the EOF token
// just like we allow the code to be indented to a particular initial
// indent level, we allow the final indent to be anything and set
@ -527,8 +519,6 @@ struct Lexer {
next_tokens.push_back(std::move(r));
}
Token lexRaw(bool whitespace_token = false) {
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
int kind;
AT_ASSERT(source);
if (current == nullptr) {
AT_ASSERT(pos == 0);
@ -538,6 +528,7 @@ struct Lexer {
StringCordView::Iterator start_iter = *current;
StringCordView::Iterator end_iter = *current;
int kind = 0;
if (!shared.match(
*current,
nesting > 0,
@ -562,11 +553,10 @@ struct Lexer {
std::unique_ptr<StringCordView::Iterator> current;
size_t pos{0};
size_t nesting{0}; // depth of ( [ { nesting...
std::vector<int> indent_stack; // stack of indentation level of blocks
std::vector<size_t> indent_stack; // stack of indentation level of blocks
// Invariant: this should always contain at least a single element
std::vector<Token> next_tokens;
// NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members)
SharedParserData& shared;
};
} // namespace torch::jit
C10_CLANG_DIAGNOSTIC_POP()

View File

@ -8,7 +8,10 @@ namespace torch::jit {
inline bool isCharCount(char c, const std::string& str, size_t start, int len) {
// count checks from [start, start + len)
return start + len <= str.size() &&
std::count(str.begin() + start, str.begin() + start + len, c) == len;
std::count(
str.begin() + static_cast<ptrdiff_t>(start),
str.begin() + static_cast<ptrdiff_t>(start + len),
c) == len;
}
inline std::optional<char> parseOctal(const std::string& str, size_t pos) {
@ -17,8 +20,7 @@ inline std::optional<char> parseOctal(const std::string& str, size_t pos) {
return std::nullopt;
size_t c = 0;
for (size_t i = 1, b = 64; i < 4; ++i, b /= 8) {
// NOLINTNEXTLINE(bugprone-signed-char-misuse)
int d = str[pos + i];
auto d = str[pos + i];
if (d < '0' || d > '7')
return std::nullopt;
c += b * (d - '0');
@ -31,7 +33,7 @@ inline std::optional<char> parseOctal(const std::string& str, size_t pos) {
inline std::string parseStringLiteral(
const SourceRange& range,
const std::string& str) {
int quote_len = isCharCount(str[0], str, 0, 3) ? 3 : 1;
size_t quote_len = isCharCount(str[0], str, 0, 3) ? 3 : 1;
auto ret_str = str.substr(quote_len, str.size() - quote_len * 2);
size_t pos = ret_str.find('\\');
while (pos != std::string::npos) {
@ -63,17 +65,17 @@ inline std::string parseStringLiteral(
c = '\t';
break;
case 'x':
throw ErrorReport(range) << "unsupported hex specifier";
throw(ErrorReport(range) << "unsupported hex specifier");
case 'u':
case 'U':
throw ErrorReport(range) << "unsupported unicode specifier";
throw(ErrorReport(range) << "unsupported unicode specifier");
default:
// octal value in format \nnn, n is [0-7]
if (auto v = parseOctal(ret_str, pos)) {
to_erase = 4;
c = *v;
} else {
throw ErrorReport(range) << " ill formed octal specifier";
throw(ErrorReport(range) << " ill formed octal specifier");
}
}
ret_str.replace(pos, to_erase, /* num copies */ 1, c);

View File

@ -263,8 +263,7 @@ struct ParserImpl {
}
Expr parseExp(int precedence) {
TreeRef prefix;
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
int unary_prec;
int unary_prec = 0;
if (shared.isUnary(L.cur().kind, &unary_prec)) {
auto kind = L.cur().kind;
auto pos = L.cur().range;
@ -283,8 +282,7 @@ struct ParserImpl {
} else {
prefix = parseBaseExp();
}
// NOLINTNEXTLINE(cppcoreguidelines-init-variables)
int binary_prec;
int binary_prec = 0;
while (shared.isBinary(L.cur().kind, &binary_prec)) {
if (binary_prec <= precedence) // not allowed to parse something which is
// not greater than 'precedence'

View File

@ -687,7 +687,6 @@ Value* emitBuiltinCall(
// first let's set the graph's version
auto graph_version = graph.get_op_version();
std::stringstream failure_messages;
std::vector<const FunctionSchema*> schemas;
// we append them later to schemas because
// parseSchema returns rvalue which can not

View File

@ -1,5 +1,4 @@
#pragma once
#include <functional>
#include <memory>
#include <optional>
#include <string>
@ -33,7 +32,7 @@ struct TORCH_API SugaredValue
// what can we do with this thing?
// use it as a value e.g. `this + 4`
virtual Value* asValue(const SourceRange& loc, GraphFunction& m) {
throw ErrorReport(loc) << kind() << " cannot be used as a value";
throw(ErrorReport(loc) << kind() << " cannot be used as a value");
}
// select an attribute on it, e.g. `this.field`
@ -41,14 +40,14 @@ struct TORCH_API SugaredValue
const SourceRange& loc,
GraphFunction& m,
const std::string& field) {
throw ErrorReport(loc) << "attribute lookup is not defined on " << kind();
throw(ErrorReport(loc) << "attribute lookup is not defined on " << kind());
}
virtual bool hasAttr(
const SourceRange& loc,
GraphFunction& m,
const std::string& field) {
throw ErrorReport(loc) << "attribute lookup is not defined on " << kind();
throw(ErrorReport(loc) << "attribute lookup is not defined on " << kind());
}
// assign an attribute on it, e.g. `this.field = newValue`
@ -57,8 +56,9 @@ struct TORCH_API SugaredValue
GraphFunction& m,
const std::string& field,
Value* newValue) {
throw ErrorReport(loc) << "attribute assignment is not defined on "
<< kind();
throw(
ErrorReport(loc) << "attribute assignment is not defined on "
<< kind());
}
// use it as a vector of values, e.g. a tuple of values as return value from
@ -67,20 +67,20 @@ struct TORCH_API SugaredValue
const SourceRange& loc,
GraphFunction& m,
const std::optional<size_t>& size_hint = {}) {
throw ErrorReport(loc) << kind() << " cannot be used as a tuple";
throw(ErrorReport(loc) << kind() << " cannot be used as a tuple");
}
// TODO @wconstab refactor to use ModuleValue::asTuple instead of new API
virtual SugaredValuePtr asTupleValue(
const SourceRange& loc,
GraphFunction& m) {
throw ErrorReport(loc) << kind() << " cannot be used as a tuplevalue";
throw(ErrorReport(loc) << kind() << " cannot be used as a tuplevalue");
}
virtual std::vector<std::shared_ptr<SugaredValue>> asType(
const SourceRange& loc,
Method& m) {
throw ErrorReport(loc) << kind() << " cannot be used as a type";
throw(ErrorReport(loc) << kind() << " cannot be used as a type");
}
// call it like a function, e.g. `outputs = this(inputs)`
@ -105,7 +105,7 @@ struct TORCH_API SugaredValue
// check that n_binders match the number of things they are returning, the
// assignment logic will do that anyway.
throw ErrorReport(loc) << "cannot call a " << kind();
throw(ErrorReport(loc) << "cannot call a " << kind());
}
// This function is called when to convert a SugaredValue to its iterator.
@ -113,7 +113,7 @@ struct TORCH_API SugaredValue
virtual std::shared_ptr<SugaredValue> iter(
const SourceRange& loc,
GraphFunction& m) {
throw ErrorReport(loc) << kind() << " cannot be used as an iterable";
throw(ErrorReport(loc) << kind() << " cannot be used as an iterable");
}
// If we are iterating over a Sugared Value and it returns a value from this
@ -135,8 +135,9 @@ struct TORCH_API SugaredValue
// be iterated over with a modulelist. If it does it must return a constant
// Value *
virtual Value* len(const SourceRange& loc, GraphFunction& m) {
throw ErrorReport(loc) << "'" << kind() << "'"
<< " object is not iterable";
throw(
ErrorReport(loc) << "'" << kind() << "'"
<< " object is not iterable");
}
// expression for ith elemement for iterable value
@ -145,8 +146,9 @@ struct TORCH_API SugaredValue
GraphFunction& m,
Value* idx,
TypePtr type_hint = nullptr) {
throw ErrorReport(loc) << "'" << kind() << "'"
<< " object is not subscriptable";
throw(
ErrorReport(loc) << "'" << kind() << "'"
<< " object is not subscriptable");
}
virtual ~SugaredValue() = default;
@ -269,18 +271,20 @@ struct TORCH_API SugaredTupleValue : public SugaredValue {
Value* idx,
TypePtr type_hint = nullptr) override {
if (!(idx->type()->cast<IntType>() && toIValue(idx))) {
throw ErrorReport(loc)
throw(
ErrorReport(loc)
<< "Expected integer literal for index but got a variable or non-integer. "
<< "ModuleList/Sequential indexing is only supported with integer literals. "
<< "For example, 'i = 4; self.layers[i](x)' will fail because i is not a literal. "
<< "Enumeration is supported, e.g. 'for index, v in enumerate(self): out = v(inp)'";
<< "Enumeration is supported, e.g. 'for index, v in enumerate(self): out = v(inp)'");
}
auto index = toIValue(idx)->toInt();
int64_t adj_index =
(index < 0) ? index + static_cast<int64_t>(tup_.size()) : index;
if (!(adj_index >= 0 && adj_index < static_cast<int64_t>(tup_.size()))) {
throw ErrorReport(loc)
<< "Index " << index << " out of range of length " << tup_.size();
throw(
ErrorReport(loc) << "Index " << index << " out of range of length "
<< tup_.size());
}
return tup_.at(adj_index);
}
@ -402,9 +406,10 @@ struct FunctionValue : public SugaredValue {
try {
callee->ensure_defined();
} catch (const RecursiveMethodCallError&) {
throw ErrorReport(loc)
throw(
ErrorReport(loc)
<< " function '" << callee->name() << "' is called recursively. "
<< "Recursive calls are not supported";
<< "Recursive calls are not supported");
}
schemas.push_back(&callee->getSchema());
}
@ -464,9 +469,10 @@ struct MethodValue : public SugaredValue {
try {
method.ensure_defined();
} catch (const RecursiveMethodCallError&) {
throw ErrorReport(loc)
throw(
ErrorReport(loc)
<< " method '" << method.name() << "' is called recursively. "
<< "Recursive calls are not supported";
<< "Recursive calls are not supported");
}
schemas.push_back(&method.getSchema());
} else if (auto interface_type = self_->type()->cast<InterfaceType>()) {

View File

@ -200,7 +200,7 @@ struct Maybe : public TreeView {
explicit Maybe(const TreeRef& tree) : TreeView(tree) {
tree_->match(TK_OPTION);
if (tree_->trees().size() > 1)
throw ErrorReport(tree) << "Maybe trees can have at most one subtree";
throw(ErrorReport(tree) << "Maybe trees can have at most one subtree");
}
/* implicit */ Maybe(const T& tree) : TreeView(tree) {}
bool present() const {
@ -258,8 +258,9 @@ struct Stmt : public TreeView {
case TK_WITH:
return;
default:
throw ErrorReport(tree)
<< kindToString(tree->kind()) << " is not a valid Stmt";
throw(
ErrorReport(tree)
<< kindToString(tree->kind()) << " is not a valid Stmt");
}
}
};
@ -317,8 +318,9 @@ struct Expr : public TreeView {
case TK_WITH_ITEM:
return;
default:
throw ErrorReport(tree)
<< kindToString(tree->kind()) << " is not a valid Expr";
throw(
ErrorReport(tree)
<< kindToString(tree->kind()) << " is not a valid Expr");
}
}
};
@ -659,7 +661,7 @@ struct AugAssignKind : public TreeView {
case TK_RSHIFT:
return;
default:
throw ErrorReport(tree) << "is not a valid AugAssignKind";
throw(ErrorReport(tree) << "is not a valid AugAssignKind");
}
}
};
@ -841,12 +843,14 @@ struct BinOp : public Expr {
case TK_FLOOR_DIV:
case TK_IN:
if (tree->trees().size() != 2)
throw ErrorReport(tree)
<< "BinOp expected 2 subtrees, found " << tree->trees().size();
throw(
ErrorReport(tree)
<< "BinOp expected 2 subtrees, found " << tree->trees().size());
return;
default:
throw ErrorReport(tree)
<< kindToString(tree->kind()) << " is not a valid BinOp";
throw(
ErrorReport(tree)
<< kindToString(tree->kind()) << " is not a valid BinOp");
}
}
Expr lhs() const {
@ -871,12 +875,14 @@ struct UnaryOp : public Expr {
case '~':
case TK_NOT:
if (tree->trees().size() != 1)
throw ErrorReport(tree)
<< "UnaryOp expected 1 subtree, found " << tree->trees().size();
throw(
ErrorReport(tree)
<< "UnaryOp expected 1 subtree, found " << tree->trees().size());
return;
default:
throw ErrorReport(tree)
<< kindToString(tree->kind()) << " is not a valid UnaryOp";
throw(
ErrorReport(tree)
<< kindToString(tree->kind()) << " is not a valid UnaryOp");
}
}
static UnaryOp create(const SourceRange& range, int kind, const Expr& expr) {
@ -904,11 +910,11 @@ struct Const : public Expr {
}
int64_t asIntegral() const {
try {
// NOLINTNEXTLINE(modernize-use-nullptr)
return std::stoll(subtree(0)->stringValue(), /*__idx=*/0, /*base=*/0);
return std::stoll(subtree(0)->stringValue(), nullptr, 0);
} catch (const std::out_of_range&) {
throw ErrorReport(range()) << "Integral constant out of range "
"(must fit in a signed 64 bit integer)";
throw(
ErrorReport(range()) << "Integral constant out of range "
"(must fit in a signed 64 bit integer)");
}
}
double asFloatingPoint() const {