Better error message for using Python builtin_function_or_method (#22935)

davidriazati · facebook-github-bot · commit ef36046ad747 · 2019-07-16T16:49:04.000-07:00
Summary: * better error in `toSugaredValue` * removes a bunch of periods on error messages, `ErrorReport` already adds a `:` at the end of the message](https://our.intern.facebook.com/intern/diff/16291079/) Pull Request resolved: #22935 Pulled By: driazati Differential Revision: D16291079 fbshipit-source-id: 478724fc7d1ae79093f4ede18553ffeafa2c7964
diff --git a/test/test_jit.py b/test/test_jit.py
@@ -10782,7 +10782,7 @@ def test_wrong_type():
 
     def test_unsupported_builtin_error(self):
         with self.assertRaisesRegex(RuntimeError,
-                                    "calling a python builtin_function_or_method which is currently not supported"):
+                                    "Python builtin <built-in function hypot> is currently"):
             @torch.jit.script
             def test_unsupported(a):
                 return math.hypot(a, 2.0)
diff --git a/torch/csrc/jit/attributes.h b/torch/csrc/jit/attributes.h
@@ -123,7 +123,7 @@ struct AttributeError : public std::exception {
     std::stringstream ss;
     if (!defined) {
       ss << "required keyword attribute '" << name.toUnqualString()
-         << "' is undefined.";
+         << "' is undefined";
     } else {
       ss << "required keyword attribute '" << name.toUnqualString()
          << "' has the wrong type";
diff --git a/torch/csrc/jit/import_source.cpp b/torch/csrc/jit/import_source.cpp
@@ -93,7 +93,7 @@ struct ConstantTableValue : public SugaredValue {
     if (offset < 0 || size_t(offset) >= constants_.size()) {
       throw ErrorReport(loc) << "constant index " << offset
                              << " is out of bounds (constant table has "
-                             << constants_.size() << " entries).";
+                             << constants_.size() << " entries)";
     }
     Value* value = m.graph()->insertConstant(constants_[offset], nullptr, loc);
     return std::make_shared<SimpleValue>(value);
@@ -176,7 +176,7 @@ struct SourceImporter {
       throw ErrorReport(p_.lexer().cur().range)
           << "Attempting to load a script generated from a newer version of PyTorch. Maximum supported TorchScript version is "
           << CURRENT_OP_VERSION_SET
-          << " but the script being loaded is version " << version_ << ".";
+          << " but the script being loaded is version " << version_;
     }
   }
 
diff --git a/torch/csrc/jit/passes/python_print.cpp b/torch/csrc/jit/passes/python_print.cpp
@@ -650,7 +650,7 @@ struct PythonPrintPass {
       throw script::ErrorReport(stmt.node()->sourceRange())
           << "loop cannot be printed as python "
           << "because it has gone through an optimization "
-          << "that combined while and for loops. File a bug.";
+          << "that combined while and for loops. File a bug";
     }
 
     bool emit_as_for_loop = loop_type == LoopView::For;
@@ -786,7 +786,7 @@ struct PythonPrintPass {
         if (enforce_importable_ && node->inputs().size() != 1) {
           throw script::ErrorReport(node->sourceRange())
               << "Exportable methods must have a single return value. "
-              << "Normal use of ScriptMethods should enforce this.";
+              << "Normal use of ScriptMethods should enforce this";
         }
         if (node->inputs().size() > 0) {
           indent();
diff --git a/torch/csrc/jit/passes/shape_analysis.cpp b/torch/csrc/jit/passes/shape_analysis.cpp
@@ -170,7 +170,7 @@ class ShapePropagator {
     // prevented it
     std::stringstream ss;
     ss << "unable to create representative value for: " << type_->str()
-       << ". File a bug report.";
+       << ". File a bug report";
     throw std::runtime_error(ss.str());
   }
 
diff --git a/torch/csrc/jit/script/compiler.cpp b/torch/csrc/jit/script/compiler.cpp
@@ -1362,7 +1362,7 @@ struct to_ir {
     auto body = stmt.body();
     if (stmt.itrs().size() != 1) {
       throw ErrorReport(stmt)
-          << "List of iterables is not supported currently.";
+          << "List of iterables is not supported currently";
     }
     // Emit loop information for builtinFunction values like range(), zip(),
     // enumerate() or SimpleValue like List, Tensor, Dict, etc.
@@ -1464,19 +1464,19 @@ struct to_ir {
         num_starred++;
       } else {
         throw ErrorReport(assignee) << "lhs of assignment must be a variable, "
-                                    << "subscript, or starred expression.";
+                                    << "subscript, or starred expression";
       }
     }
 
     if (num_starred > 1) {
       throw ErrorReport(r)
-          << "Only one starred expression is allowed on the lhs.";
+          << "Only one starred expression is allowed on the lhs";
     }
 
     if (num_starred > 0 && num_normal_assign == 0) {
       throw ErrorReport(r) << "A Starred expression may only appear on the "
                            << "lhs within the presence of another non-starred"
-                           << " expression.";
+                           << " expression";
     }
 
     return num_starred;
@@ -1523,7 +1523,7 @@ struct to_ir {
       default:
         throw ErrorReport(stmt.lhs())
             << "unexpected expression on "
-            << "left-hand side of augmented assignment.";
+            << "left-hand side of augmented assignment";
     }
   }
 
@@ -1665,7 +1665,7 @@ struct to_ir {
         throw ErrorReport(subscriptExprs)
             << "Sliced expression not yet supported for"
             << " subscripted list augmented assignment. "
-            << "File a bug if you want this.";
+            << "File a bug if you want this";
       }
       const auto idxValue = emitExpr(subscriptExprs[0]);
 
@@ -1735,7 +1735,7 @@ struct to_ir {
         throw ErrorReport(subscript)
             << "Sliced expression not yet supported for"
             << " subscripted list assignment. "
-            << "File a bug if you want this.";
+            << "File a bug if you want this";
       }
 
       std::vector<NamedValue> args;
@@ -1798,7 +1798,7 @@ struct to_ir {
           auto var = Starred(assignee).expr();
           if (var.kind() != TK_VAR) {
             throw ErrorReport(var)
-                << "Cannot pack a tuple into a non-variable.";
+                << "Cannot pack a tuple into a non-variable";
           }
           size_t n_matched = outputs.size() - n_binders;
           ArrayRef<std::shared_ptr<SugaredValue>> outputs_ref = outputs;
@@ -1850,7 +1850,7 @@ struct to_ir {
         break;
       default:
         throw ErrorReport(stmt.lhs())
-            << "unexpected expression on left-hand side of assignment.";
+            << "unexpected expression on left-hand side of assignment";
     }
   }
 
@@ -2443,7 +2443,7 @@ struct to_ir {
       }
       case TK_STARRED: {
         throw ErrorReport(tree)
-            << "Unexpected starred expansion. File a bug report.";
+            << "Unexpected starred expansion. File a bug report";
       }
       case TK_CONST: {
         return emitConst(Const(tree));
@@ -2751,7 +2751,7 @@ struct to_ir {
     if (!sliceable->type()->isSubtypeOf(TensorType::get())) {
       throw ErrorReport(loc)
           << "Unsupported operation: attempted to use multidimensional "
-          << "indexing on a non-tensor type.";
+          << "indexing on a non-tensor type";
     }
 
     std::vector<Value*> tensor_indices;
diff --git a/torch/csrc/jit/script/final_returns.cpp b/torch/csrc/jit/script/final_returns.cpp
@@ -13,7 +13,7 @@ struct ReturnInfo {
 
 void checkNoReturn(const TreeRef& ref) {
   if (ref->kind() == TK_RETURN) {
-    throw ErrorReport(ref) << "return is not allowed from a loop.";
+    throw ErrorReport(ref) << "return is not allowed from a loop";
   }
   // do not search into first-class functions
   if (ref->kind() == TK_DEF) {
@@ -56,7 +56,7 @@ ReturnInfo makeReturnsFinal(
           if (!rest_final.returns_) {
             throw ErrorReport(if_stmt)
                 << "This if statement performs an early return, but the block of code that follows it does not return."
-                << " Early returns are only allowed when the block following them also returns.";
+                << " Early returns are only allowed when the block following them also returns";
           }
           changed.emplace_back(
               if_stmt.withNewBranches(true_final.stmts_, rest_final.stmts_));
@@ -77,7 +77,7 @@ ReturnInfo makeReturnsFinal(
         }
         throw ErrorReport(if_stmt)
             << "This if statement contains some paths that return and some paths that do not. "
-            << "If statements must either entirely return or never return.";
+            << "If statements must either entirely return or never return";
       } break;
       case TK_WHILE:
       case TK_FOR:
diff --git a/torch/csrc/jit/script/function_schema_parser.cpp b/torch/csrc/jit/script/function_schema_parser.cpp
@@ -194,7 +194,7 @@ struct SchemaParser {
         return c10::impl::toList(fmap(vs, [](IValue v) { return v.toBool(); }));
       default:
         throw ErrorReport(range)
-            << "lists are only supported for float or int types.";
+            << "lists are only supported for float or int types";
     }
   }
   IValue parseConstantList(TypeKind kind) {
diff --git a/torch/csrc/jit/script/python_sugared_value.cpp b/torch/csrc/jit/script/python_sugared_value.cpp
@@ -487,10 +487,8 @@ std::shared_ptr<SugaredValue> toSugaredValue(
 
   if (py::isinstance<py::function>(obj)) {
     if (typeString(obj) == "builtin_function_or_method") {
-      throw ErrorReport(loc)
-          << "You are calling a python builtin_function_or_method "
-          << "which is currently not supported in Torchscript."
-          << "Please open a feature request to add it.";
+      throw ErrorReport(loc) << "Python builtin " << py::str(obj)
+                             << " is currently not supported in Torchscript";
     }
   }
 
diff --git a/torch/csrc/jit/script/sugared_value.cpp b/torch/csrc/jit/script/sugared_value.cpp
@@ -207,7 +207,7 @@ void SimpleValue::setAttr(
       if (insertPoint->owningBlock() != topLevelBlock) {
         throw ErrorReport(loc)
             << "First assignment cannot be in a control-flow block. "
-            << "Initialize the field at the top level first.";
+            << "Initialize the field at the top level first";
       }
     } else {
       throw ErrorReport(loc)
@@ -407,7 +407,7 @@ std::shared_ptr<SugaredValue> ClassValue::call(
   auto self = g.insertNode(g.createObject(type_))->output();
   if (!type_->getMethod("__init__")) {
     throw ErrorReport(loc)
-        << "Class " << type_->basename() << " does not have an __init__ function defined.";
+        << "Class " << type_->basename() << " does not have an __init__ function defined";
   }
 
   // Call the init function

Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ struct ConstantTableValue : public SugaredValue {`
`93`	`93`	`if (offset < 0 \|\| size_t(offset) >= constants_.size()) {`
`94`	`94`	`throw ErrorReport(loc) << "constant index " << offset`
`95`	`95`	`<< " is out of bounds (constant table has "`
`96`		`- << constants_.size() << " entries).";`
	`96`	`+ << constants_.size() << " entries)";`
`97`	`97`	`}`
`98`	`98`	`Value* value = m.graph()->insertConstant(constants_[offset], nullptr, loc);`
`99`	`99`	`return std::make_shared<SimpleValue>(value);`
`@@ -176,7 +176,7 @@ struct SourceImporter {`
`176`	`176`	`throw ErrorReport(p_.lexer().cur().range)`
`177`	`177`	`<< "Attempting to load a script generated from a newer version of PyTorch. Maximum supported TorchScript version is "`
`178`	`178`	`<< CURRENT_OP_VERSION_SET`
`179`		`- << " but the script being loaded is version " << version_ << ".";`
	`179`	`+ << " but the script being loaded is version " << version_;`
`180`	`180`	`}`
`181`	`181`	`}`
`182`	`182`
Original file line number	Diff line number	Diff line change
`@@ -170,7 +170,7 @@ class ShapePropagator {`
`170`	`170`	`// prevented it`
`171`	`171`	`std::stringstream ss;`
`172`	`172`	`ss << "unable to create representative value for: " << type_->str()`
`173`		`- << ". File a bug report.";`
	`173`	`+ << ". File a bug report";`
`174`	`174`	`throw std::runtime_error(ss.str());`
`175`	`175`	`}`
`176`	`176`
Original file line number	Diff line number	Diff line change
`@@ -1362,7 +1362,7 @@ struct to_ir {`
`1362`	`1362`	`auto body = stmt.body();`
`1363`	`1363`	`if (stmt.itrs().size() != 1) {`
`1364`	`1364`	`throw ErrorReport(stmt)`
`1365`		`- << "List of iterables is not supported currently.";`
	`1365`	`+ << "List of iterables is not supported currently";`
`1366`	`1366`	`}`
`1367`	`1367`	`// Emit loop information for builtinFunction values like range(), zip(),`
`1368`	`1368`	`// enumerate() or SimpleValue like List, Tensor, Dict, etc.`
`@@ -1464,19 +1464,19 @@ struct to_ir {`
`1464`	`1464`	`num_starred++;`
`1465`	`1465`	`} else {`
`1466`	`1466`	`throw ErrorReport(assignee) << "lhs of assignment must be a variable, "`
`1467`		`- << "subscript, or starred expression.";`
	`1467`	`+ << "subscript, or starred expression";`
`1468`	`1468`	`}`
`1469`	`1469`	`}`
`1470`	`1470`
`1471`	`1471`	`if (num_starred > 1) {`
`1472`	`1472`	`throw ErrorReport(r)`
`1473`		`- << "Only one starred expression is allowed on the lhs.";`
	`1473`	`+ << "Only one starred expression is allowed on the lhs";`
`1474`	`1474`	`}`
`1475`	`1475`
`1476`	`1476`	`if (num_starred > 0 && num_normal_assign == 0) {`
`1477`	`1477`	`throw ErrorReport(r) << "A Starred expression may only appear on the "`
`1478`	`1478`	`<< "lhs within the presence of another non-starred"`
`1479`		`- << " expression.";`
	`1479`	`+ << " expression";`
`1480`	`1480`	`}`
`1481`	`1481`
`1482`	`1482`	`return num_starred;`
`@@ -1523,7 +1523,7 @@ struct to_ir {`
`1523`	`1523`	`default:`
`1524`	`1524`	`throw ErrorReport(stmt.lhs())`
`1525`	`1525`	`<< "unexpected expression on "`
`1526`		`- << "left-hand side of augmented assignment.";`
	`1526`	`+ << "left-hand side of augmented assignment";`
`1527`	`1527`	`}`
`1528`	`1528`	`}`
`1529`	`1529`
`@@ -1665,7 +1665,7 @@ struct to_ir {`
`1665`	`1665`	`throw ErrorReport(subscriptExprs)`
`1666`	`1666`	`<< "Sliced expression not yet supported for"`
`1667`	`1667`	`<< " subscripted list augmented assignment. "`
`1668`		`- << "File a bug if you want this.";`
	`1668`	`+ << "File a bug if you want this";`
`1669`	`1669`	`}`
`1670`	`1670`	`const auto idxValue = emitExpr(subscriptExprs[0]);`
`1671`	`1671`
`@@ -1735,7 +1735,7 @@ struct to_ir {`
`1735`	`1735`	`throw ErrorReport(subscript)`
`1736`	`1736`	`<< "Sliced expression not yet supported for"`
`1737`	`1737`	`<< " subscripted list assignment. "`
`1738`		`- << "File a bug if you want this.";`
	`1738`	`+ << "File a bug if you want this";`
`1739`	`1739`	`}`
`1740`	`1740`
`1741`	`1741`	`std::vector<NamedValue> args;`
`@@ -1798,7 +1798,7 @@ struct to_ir {`
`1798`	`1798`	`auto var = Starred(assignee).expr();`
`1799`	`1799`	`if (var.kind() != TK_VAR) {`
`1800`	`1800`	`throw ErrorReport(var)`
`1801`		`- << "Cannot pack a tuple into a non-variable.";`
	`1801`	`+ << "Cannot pack a tuple into a non-variable";`
`1802`	`1802`	`}`
`1803`	`1803`	`size_t n_matched = outputs.size() - n_binders;`
`1804`	`1804`	`ArrayRef<std::shared_ptr<SugaredValue>> outputs_ref = outputs;`
`@@ -1850,7 +1850,7 @@ struct to_ir {`
`1850`	`1850`	`break;`
`1851`	`1851`	`default:`
`1852`	`1852`	`throw ErrorReport(stmt.lhs())`
`1853`		`- << "unexpected expression on left-hand side of assignment.";`
	`1853`	`+ << "unexpected expression on left-hand side of assignment";`
`1854`	`1854`	`}`
`1855`	`1855`	`}`
`1856`	`1856`
`@@ -2443,7 +2443,7 @@ struct to_ir {`
`2443`	`2443`	`}`
`2444`	`2444`	`case TK_STARRED: {`
`2445`	`2445`	`throw ErrorReport(tree)`
`2446`		`- << "Unexpected starred expansion. File a bug report.";`
	`2446`	`+ << "Unexpected starred expansion. File a bug report";`
`2447`	`2447`	`}`
`2448`	`2448`	`case TK_CONST: {`
`2449`	`2449`	`return emitConst(Const(tree));`
`@@ -2751,7 +2751,7 @@ struct to_ir {`
`2751`	`2751`	`if (!sliceable->type()->isSubtypeOf(TensorType::get())) {`
`2752`	`2752`	`throw ErrorReport(loc)`
`2753`	`2753`	`<< "Unsupported operation: attempted to use multidimensional "`
`2754`		`- << "indexing on a non-tensor type.";`
	`2754`	`+ << "indexing on a non-tensor type";`
`2755`	`2755`	`}`
`2756`	`2756`
`2757`	`2757`	`std::vector<Value*> tensor_indices;`
Original file line number	Diff line number	Diff line change
`@@ -194,7 +194,7 @@ struct SchemaParser {`
`194`	`194`	`return c10::impl::toList(fmap(vs, [](IValue v) { return v.toBool(); }));`
`195`	`195`	`default:`
`196`	`196`	`throw ErrorReport(range)`
`197`		`- << "lists are only supported for float or int types.";`
	`197`	`+ << "lists are only supported for float or int types";`
`198`	`198`	`}`
`199`	`199`	`}`
`200`	`200`	`IValue parseConstantList(TypeKind kind) {`
Original file line number	Diff line number	Diff line change
`@@ -487,10 +487,8 @@ std::shared_ptr<SugaredValue> toSugaredValue(`
`487`	`487`
`488`	`488`	`if (py::isinstance<py::function>(obj)) {`
`489`	`489`	`if (typeString(obj) == "builtin_function_or_method") {`
`490`		`- throw ErrorReport(loc)`
`491`		`- << "You are calling a python builtin_function_or_method "`
`492`		`- << "which is currently not supported in Torchscript."`
`493`		`- << "Please open a feature request to add it.";`
	`490`	`+ throw ErrorReport(loc) << "Python builtin " << py::str(obj)`
	`491`	`+ << " is currently not supported in Torchscript";`
`494`	`492`	`}`
`495`	`493`	`}`
`496`	`494`