From cbaebc9189791c343b84ded8409a5c5fd7ecde8f Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Thu, 21 Oct 2021 21:17:58 -0400 Subject: [PATCH 01/14] writing tests for is keyword --- tuplex/codegen/include/TokenType.h | 1 + tuplex/codegen/src/BlockGeneratorVisitor.cc | 46 +++- tuplex/codegen/src/Token.cc | 9 +- tuplex/core/src/TraceVisitor.cc | 5 + tuplex/python/tests/test_is.py | 26 +++ tuplex/test/core/IsKeywordTest.cc | 219 ++++++++++++++++++++ tuplex/test/core/PythonTracer.cc | 29 +++ 7 files changed, 327 insertions(+), 8 deletions(-) create mode 100644 tuplex/python/tests/test_is.py create mode 100644 tuplex/test/core/IsKeywordTest.cc diff --git a/tuplex/codegen/include/TokenType.h b/tuplex/codegen/include/TokenType.h index 6be07ecfb..f6f4e217a 100644 --- a/tuplex/codegen/include/TokenType.h +++ b/tuplex/codegen/include/TokenType.h @@ -51,6 +51,7 @@ enum class TokenType { IN, NOTIN, IS, + ISNOT, LAMBDA, NONLOCAL, NOT, diff --git a/tuplex/codegen/src/BlockGeneratorVisitor.cc b/tuplex/codegen/src/BlockGeneratorVisitor.cc index 7645bf350..e1b757874 100644 --- a/tuplex/codegen/src/BlockGeneratorVisitor.cc +++ b/tuplex/codegen/src/BlockGeneratorVisitor.cc @@ -957,6 +957,35 @@ namespace tuplex { assert(!leftType.isOptional()); assert(!rightType.isOptional()); + if(tt == TokenType::IS || tt == TokenType::ISNOT) { + bool invertResult = (tt == TokenType::ISNOT); + + std::unordered_set validTypes = {python::Type::BOOLEAN, python::Type::NULLVALUE}; + if (!(validTypes.count(leftType) && validTypes.count(rightType))) { + std::stringstream ss; + ss << "Could not generate is comparison for types " + << leftType.desc() + << " " << opToString(tt) << " " + << rightType.desc(); + error(ss.str()); + // return TRUE as dummy constant to continue tracking process + return _env->boolConst(true); + } + + if(leftType == python::Type::NULLVALUE && rightType == python::Type::NULLVALUE) { + return _env->boolConst(!invertResult); + } + + // comparison of None with boolean is always False. + if(leftType != rightType) { + return _env->boolConst(invertResult); + } + + // at this point we are doing an is comparison between booleans. + return invertResult ? _env->upcastToBoolean(builder, builder.CreateICmp(llvm::CmpInst::Predicate::ICMP_NE, L, R)) + : _env->upcastToBoolean(builder, builder.CreateICmp(llvm::CmpInst::Predicate::ICMP_EQ, L, R)); + } + assert(L); assert(R); // comparison of values without null @@ -1125,15 +1154,18 @@ namespace tuplex { return listInclusionCheck(builder, L, leftType, R, rightType.withoutOptions()); } else { // exception check left - if (L_isnull) { - _lfb->addException(builder, ExceptionCode::TYPEERROR, L_isnull); - } - // exception check right - if (R_isnull) { - _lfb->addException(builder, ExceptionCode::TYPEERROR, R_isnull); - } + if(tt != TokenType::IS && tt != TokenType::ISNOT) { + if (L_isnull) { + _lfb->addException(builder, ExceptionCode::TYPEERROR, L_isnull); + } + // exception check right + if (R_isnull) { + _lfb->addException(builder, ExceptionCode::TYPEERROR, R_isnull); + } + } + auto resVal = compareInst(builder, L, leftType.withoutOptions(), tt, R, rightType.withoutOptions()); _lfb->setLastBlock(builder.GetInsertBlock()); return resVal; diff --git a/tuplex/codegen/src/Token.cc b/tuplex/codegen/src/Token.cc index 050d8dbd6..450ebc28e 100644 --- a/tuplex/codegen/src/Token.cc +++ b/tuplex/codegen/src/Token.cc @@ -45,6 +45,7 @@ std::ostream& operator<< (std::ostream& os, const TokenType tt) case TokenType::IMPORT: return os<<"IMPORT"; case TokenType::IN: return os<<"IN"; case TokenType::IS: return os<<"IS"; + case TokenType::ISNOT: return os<<"ISNOT"; case TokenType::LAMBDA: return os<<"LAMBDA"; case TokenType::NONLOCAL: return os<<"NONLOCAL"; case TokenType::NOT: return os<<"NOT"; @@ -112,7 +113,10 @@ TokenType stringToToken(const std::string& s) { return TokenType::IN; if(s == "notin") return TokenType::NOTIN; - + if(s == "is") + return TokenType::IS; + if(s == "isnot") + return TokenType::ISNOT; if(s == "&&" || s == "and") return TokenType::AND; if(s == "!") @@ -267,6 +271,9 @@ std::string opToString(const TokenType& tt) { case TokenType::DOUBLESLASHEQUAL: return "//="; case TokenType::ELLIPSIS: return "..."; case TokenType::IN: return "in"; + case TokenType::IS: return "is"; + case TokenType::ISNOT: return "is not"; + default: return std::to_string(static_cast(tt)); } } \ No newline at end of file diff --git a/tuplex/core/src/TraceVisitor.cc b/tuplex/core/src/TraceVisitor.cc index 05f71013c..92854453b 100644 --- a/tuplex/core/src/TraceVisitor.cc +++ b/tuplex/core/src/TraceVisitor.cc @@ -388,7 +388,11 @@ namespace tuplex { // now truth value testing, single element? auto res = ti_vals.front(); + // IS and IS NOT were added here with these op ids because as far as optimizations go, + // their functionality should be similar to EQ/NE. std::unordered_map cmpLookup{{TokenType::EQEQUAL, Py_EQ}, + {TokenType::IS, Py_EQ}, + {TokenType::ISNOT, Py_NE}, {TokenType::NOTEQUAL, Py_NE}, {TokenType::LESS, Py_LT}, {TokenType::LESSEQUAL, Py_LE}, @@ -407,6 +411,7 @@ namespace tuplex { auto info = python::PyString_AsString(res.value) + " " + opToString(op) + " " + python::PyString_AsString(ti_vals[i+1].value); + // cpython api. res.value = PyObject_RichCompare(res.value, ti_vals[i + 1].value, opid); auto res_info = "is: " + python::PyString_AsString(res.value); diff --git a/tuplex/python/tests/test_is.py b/tuplex/python/tests/test_is.py new file mode 100644 index 000000000..d3ced8779 --- /dev/null +++ b/tuplex/python/tests/test_is.py @@ -0,0 +1,26 @@ +from unittest import TestCase +import tuplex +class TestIs(TestCase): + + def setUp(self): + self.c = tuplex.Context(webui=False) + + def test_boolIsBool(self): + res = self.c.parallelize([False, True, False, False, True]).map(lambda x: x is False).collect() + self.assertEqual(res, [True, False, True, True, False]) + + def test_boolIsNotBool(self): + res = self.c.parallelize([True, False, True, False, True]).map(lambda x: x is not False).collect() + self.assertEqual(res, [True, False, True, False, True]) + + def test_boolIsNone(self): + res = self.c.parallelize([True, False, False, True]).map(lambda x: x is None).collect() + self.assertEqual(res, [False] * 4) + + def test_mixedIsNotNone(self): + res = self.c.parallelize([None, True, False]).map(lambda x: x is not None).collect() + self.assertEqual(res, [False, True, True]) + + # def test_throwError(self): + # res = self.c.parallelize(["hi", 0.5, {'a': 0}]).map(lambda x: x is not True).collect() + # self.assertRaises(TypeError) diff --git a/tuplex/test/core/IsKeywordTest.cc b/tuplex/test/core/IsKeywordTest.cc new file mode 100644 index 000000000..19769c80f --- /dev/null +++ b/tuplex/test/core/IsKeywordTest.cc @@ -0,0 +1,219 @@ + +#include "gtest/gtest.h" +#include +#include "TestUtils.h" +#include + +#include "gtest/gtest.h" +#include +#include +#include "../../codegen/include/parser/Parser.h" + +class IsKeywordTest : public PyTest {}; + +// bool saveToPDFx(tuplex::ASTNode* root, const std::string& path) { +// using namespace tuplex; +// assert(root); +// GraphVizGraph graph; +// graph.createFromAST(root); +// return graph.saveAsPDF(path); +// } + +// // works +// TEST_F(IsKeywordTest, xIsNone) { +// using namespace tuplex; +// auto code = "lambda x: x is None"; +// auto node = std::unique_ptr(tuplex::parseToAST(code)); +// ASSERT_TRUE(node); +// printParseTree(code, std::cout); +// saveToPDFx(node.get(), "xIsNone.pdf"); +// } + +TEST_F(IsKeywordTest, OptionBoolIsBool) { + using namespace tuplex; + + Context c(microTestOptions()); + Row row1(Field(option(true))); + Row row2(Field(option(true))); + Row row3(Field(option(true))); + Row row4(Field(option(true))); + + auto code = "lambda x: x is True"; + auto m = c.parallelize({row1, row2, row3, row4}) + .map(UDF(code)).collectAsVector(); + + EXPECT_EQ(m.size(), 4); + for(int i = 0; i < m.size(); i++) { + assert(m[i].toPythonString() == "(True,)"); + } +} + +TEST_F(IsKeywordTest, OptionBoolIsNotBool) { + using namespace tuplex; + + Context c(microTestOptions()); + Row row1(Field(option(true))); + Row row2(Field(option(true))); + Row row3(Field(option(true))); + Row row4(Field(option(true))); + + auto code = "lambda x: x is not True"; + auto m = c.parallelize({row1, row2, row3, row4}) + .map(UDF(code)).collectAsVector(); + + EXPECT_EQ(m.size(), 4); + for(int i = 0; i < m.size(); i++) { + assert(m[i].toPythonString() == "(False,)"); + } +} + + +TEST_F(IsKeywordTest, BoolIsBool) { + using namespace tuplex; + + Context c(microTestOptions()); + Row row1(Field(true)); + Row row2(Field(false)); + Row row3(Field(false)); + Row row4(Field(true)); + + auto code = "lambda x: x is True"; + auto m = c.parallelize({row1, row2, row3, row4}) + .map(UDF(code)).collectAsVector(); + + EXPECT_EQ(m.size(), 4); + assert(m[0].toPythonString() == "(True,)"); + assert(m[1].toPythonString() == "(False,)"); + assert(m[2].toPythonString() == "(False,)"); + assert(m[3].toPythonString() == "(True,)"); +} + +TEST_F(IsKeywordTest, BoolIsNotBool) { + using namespace tuplex; + + Context c(microTestOptions()); + Row row1(Field(true)); + Row row2(Field(false)); + Row row3(Field(false)); + Row row4(Field(true)); + + auto code = "lambda x: x is not True"; + auto m = c.parallelize({row1, row2, row3, row4}) + .map(UDF(code)).collectAsVector(); + + EXPECT_EQ(m.size(), 4); + assert(m[0].toPythonString() == "(False,)"); + assert(m[1].toPythonString() == "(True,)"); + assert(m[2].toPythonString() == "(True,)"); + assert(m[3].toPythonString() == "(False,)"); +} + + +TEST_F(IsKeywordTest, NoneIsNone) { + using namespace tuplex; + + Context c(microTestOptions()); + Row row1(Field::null()); + Row row2(Field::null()); + Row row3(Field::null()); + Row row4(Field::null()); + + auto code = "lambda x: x is None"; + auto m = c.parallelize({row1, row2, row3, row4}) + .map(UDF(code)).collectAsVector(); + + EXPECT_EQ(m.size(), 4); + for(int i = 0; i < m.size(); i++) { + assert(m[i].toPythonString() == "(True,)"); + } +} + +TEST_F(IsKeywordTest, NoneIsNotNone) { + using namespace tuplex; + + Context c(microTestOptions()); + Row row1(Field::null()); + Row row2(Field::null()); + Row row3(Field::null()); + Row row4(Field::null()); + + auto code = "lambda x: x is not None"; + auto m = c.parallelize({row1, row2, row3, row4}) + .map(UDF(code)).collectAsVector(); + + EXPECT_EQ(m.size(), 4); + for(int i = 0; i < m.size(); i++) { + assert(m[i].toPythonString() == "(False,)"); + } +} + + +TEST_F(IsKeywordTest, MixedIsNotNone) { + using namespace tuplex; + + Context c(microTestOptions()); + Row row1(Field::null()); + Row row2(Field(true)); + Row row3(Field::null()); + Row row4(Field(false)); + + auto code = "lambda x: x is not None"; + auto m = c.parallelize({row1, row2, row3, row4}) + .map(UDF(code)).collectAsVector(); + + EXPECT_EQ(m.size(), 4); + assert(m[0].toPythonString() == "(False,)"); + assert(m[1].toPythonString() == "(True,)"); + assert(m[2].toPythonString() == "(False,)"); + assert(m[3].toPythonString() == "(True,)"); +} + + +/* + Even though a std::runtime_error is thrown in BlockGeneratorVisitor.cc, it + doesn't get caught in catch(...) for some reason. +*/ + + +// TEST_F(IsKeywordTest, AnyIsNone) { +// using namespace tuplex; + +// // Logger::instance().defaultLogger().warn("asdgasdjlkgalksdgjalksdjgalksdjgas"); + +// Context c(microTestOptions()); +// Row row1(Field(option(5))); +// Row row2(Field("hi")); +// Row row3(Field(0.55)); +// Row row4(Field("0.55")); + +// auto code = "lambda x: x is None"; + +// try { +// auto udf = UDF(code); +// } catch(const std::exception& e) { +// std::string s = e.what(); +// Logger::instance().defaultLogger().error("exception string is this: " + s); +// } catch(...) { +// Logger::instance().defaultLogger().error("hmm, different exception occured"); +// } + +// auto m = c.parallelize({row1, row2, row3, row4}) +// .map(UDF(code)).collectAsVector(); + +// } + +// TEST_F(IsKeywordTest, AnyIsNotNone) { +// using namespace tuplex; + +// Context c(microTestOptions()); +// Row row1(Field(option(5))); +// Row row2(Field("hi")); +// Row row3(Field(0.55)); +// Row row4(Field("0.55")); + +// auto code = "lambda x: x is not None"; +// auto m = c.parallelize({row1, row2, row3, row4}) +// .map(UDF(code)).collectAsVector(); + +// EXPECT_EQ(m.size(), 0); +// } diff --git a/tuplex/test/core/PythonTracer.cc b/tuplex/test/core/PythonTracer.cc index 69a294cfc..f1a56f57c 100644 --- a/tuplex/test/core/PythonTracer.cc +++ b/tuplex/test/core/PythonTracer.cc @@ -89,6 +89,9 @@ TEST_F(TracerTest, ZillowExtractBd) { // cout< Date: Thu, 21 Oct 2021 22:29:33 -0400 Subject: [PATCH 02/14] update test_is.py with failing test --- tuplex/python/tests/test_is.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tuplex/python/tests/test_is.py b/tuplex/python/tests/test_is.py index d3ced8779..1e7aa5b18 100644 --- a/tuplex/python/tests/test_is.py +++ b/tuplex/python/tests/test_is.py @@ -3,6 +3,7 @@ class TestIs(TestCase): def setUp(self): + self.conf = {"webui.enable": False, "driverMemory": "64MB", "executorMemory": "2MB", "partitionSize": "128KB"} self.c = tuplex.Context(webui=False) def test_boolIsBool(self): @@ -17,7 +18,13 @@ def test_boolIsNone(self): res = self.c.parallelize([True, False, False, True]).map(lambda x: x is None).collect() self.assertEqual(res, [False] * 4) + # cannot put None here, because None becomes def test_mixedIsNotNone(self): + res = self.c.parallelize([None, None, None]).map(lambda x: x is not None).collect() + self.assertEqual(res, [False, False, False]) + + # this test fails for none, probably has to do with normal case. + def test_mixedIsNotNone2(self): res = self.c.parallelize([None, True, False]).map(lambda x: x is not None).collect() self.assertEqual(res, [False, True, True]) From 51fbd52c64ab1a86dd1b4bebf58c47d0f60d40ae Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Thu, 21 Oct 2021 22:39:40 -0400 Subject: [PATCH 03/14] fix typo --- tuplex/test/core/PythonTracer.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/tuplex/test/core/PythonTracer.cc b/tuplex/test/core/PythonTracer.cc index f1a56f57c..923ce829c 100644 --- a/tuplex/test/core/PythonTracer.cc +++ b/tuplex/test/core/PythonTracer.cc @@ -89,9 +89,6 @@ TEST_F(TracerTest, ZillowExtractBd) { // cout< Date: Fri, 22 Oct 2021 08:02:32 -0400 Subject: [PATCH 04/14] refactor and change logic, support optional types --- tuplex/codegen/src/BlockGeneratorVisitor.cc | 79 +++++++++------------ tuplex/python/tests/test_is.py | 4 -- tuplex/test/core/IsKeywordTest.cc | 34 ++++----- 3 files changed, 52 insertions(+), 65 deletions(-) diff --git a/tuplex/codegen/src/BlockGeneratorVisitor.cc b/tuplex/codegen/src/BlockGeneratorVisitor.cc index e1b757874..09a6daf7f 100644 --- a/tuplex/codegen/src/BlockGeneratorVisitor.cc +++ b/tuplex/codegen/src/BlockGeneratorVisitor.cc @@ -957,37 +957,16 @@ namespace tuplex { assert(!leftType.isOptional()); assert(!rightType.isOptional()); + assert(L); + assert(R); + if(tt == TokenType::IS || tt == TokenType::ISNOT) { + // type must be boolean, otherwise compareInst with _isnull would've taken care. bool invertResult = (tt == TokenType::ISNOT); - - std::unordered_set validTypes = {python::Type::BOOLEAN, python::Type::NULLVALUE}; - if (!(validTypes.count(leftType) && validTypes.count(rightType))) { - std::stringstream ss; - ss << "Could not generate is comparison for types " - << leftType.desc() - << " " << opToString(tt) << " " - << rightType.desc(); - error(ss.str()); - // return TRUE as dummy constant to continue tracking process - return _env->boolConst(true); - } - - if(leftType == python::Type::NULLVALUE && rightType == python::Type::NULLVALUE) { - return _env->boolConst(!invertResult); - } - - // comparison of None with boolean is always False. - if(leftType != rightType) { - return _env->boolConst(invertResult); - } - - // at this point we are doing an is comparison between booleans. return invertResult ? _env->upcastToBoolean(builder, builder.CreateICmp(llvm::CmpInst::Predicate::ICMP_NE, L, R)) - : _env->upcastToBoolean(builder, builder.CreateICmp(llvm::CmpInst::Predicate::ICMP_EQ, L, R)); - } + : _env->upcastToBoolean(builder, builder.CreateICmp(llvm::CmpInst::Predicate::ICMP_EQ, L, R)); + } - assert(L); - assert(R); // comparison of values without null auto superType = python::Type::superType(leftType.withoutOptions(), rightType.withoutOptions()); if (superType == python::Type::UNKNOWN) { @@ -1010,10 +989,10 @@ namespace tuplex { llvm::Value* BlockGeneratorVisitor::oneSidedNullComparison(llvm::IRBuilder<>& builder, const python::Type& type, const TokenType& tt, llvm::Value* isnull) { - assert(tt == TokenType::EQEQUAL || tt == TokenType::NOTEQUAL); // only for == or !=! + assert(tt == TokenType::EQEQUAL || tt == TokenType::NOTEQUAL || tt == TokenType::IS || tt == TokenType::ISNOT); // only for == or !=! if(type == python::Type::NULLVALUE) - return _env->boolConst(tt == TokenType::EQEQUAL); // if == then true, if != then false + return _env->boolConst(tt == TokenType::EQEQUAL || tt == TokenType::IS); // if == then true, if != then false // option type? check isnull // else, super simple. Decide on tokentype @@ -1025,7 +1004,7 @@ namespace tuplex { // the other side is null // if isnull is true && equal => true // if isnull is false && notequal => false (case 12 != None) - if(tt == TokenType::NOTEQUAL) + if(tt == TokenType::NOTEQUAL || tt == TokenType::ISNOT) return _env->upcastToBoolean(builder, _env->i1neg(builder, isnull)); else return _env->upcastToBoolean(builder, isnull); @@ -1033,7 +1012,7 @@ namespace tuplex { // the other side is null // => 12 != null => true // => 12 == null => false - return _env->boolConst(tt == TokenType::NOTEQUAL); + return _env->boolConst(tt == TokenType::NOTEQUAL || tt == TokenType::ISNOT); } } @@ -1043,8 +1022,23 @@ namespace tuplex { const python::Type &rightType) { // None comparisons only work for == or !=, i.e. for all other ops throw exception - if (tt == TokenType::EQEQUAL || tt == TokenType::NOTEQUAL) { + if (tt == TokenType::EQEQUAL || tt == TokenType::NOTEQUAL || tt == TokenType::IS || tt == TokenType::ISNOT) { // special case: one side is None + + if(tt == TokenType::IS || tt == TokenType::ISNOT) { + std::unordered_set validTypes = {python::Type::BOOLEAN, python::Type::NULLVALUE}; + if (!(validTypes.count(leftType.withoutOptions()) && validTypes.count(rightType.withoutOptions()))) { + std::stringstream ss; + ss << "Cannot generate is comparison for types " + << leftType.desc() + << " " << opToString(tt) << " " + << rightType.desc(); + error(ss.str()); + // return TRUE as dummy constant to continue tracking process + return _env->boolConst(true); + } + } + if(leftType == python::Type::NULLVALUE || rightType == python::Type::NULLVALUE) { // left side NULL VALUE? @@ -1072,7 +1066,7 @@ namespace tuplex { assert(L); assert(R); - auto resVal = _env->CreateTernaryLogic(builder, L_isnull, [&] (llvm::IRBuilder<>& builder) { return _env->boolConst(tt == TokenType::NOTEQUAL); }, + auto resVal = _env->CreateTernaryLogic(builder, L_isnull, [&] (llvm::IRBuilder<>& builder) { return _env->boolConst(tt == TokenType::NOTEQUAL || tt == TokenType::ISNOT); }, [&] (llvm::IRBuilder<>& builder) { return compareInst(builder, L, leftType.withoutOptions(), tt, R, rightType); }); _lfb->setLastBlock(builder.GetInsertBlock()); return resVal; @@ -1088,7 +1082,7 @@ namespace tuplex { assert(L); assert(R); - auto resVal = _env->CreateTernaryLogic(builder, R_isnull, [&] (llvm::IRBuilder<>& builder) { return _env->boolConst(tt == TokenType::NOTEQUAL); }, + auto resVal = _env->CreateTernaryLogic(builder, R_isnull, [&] (llvm::IRBuilder<>& builder) { return _env->boolConst(tt == TokenType::NOTEQUAL || tt == TokenType::ISNOT); }, [&] (llvm::IRBuilder<>& builder) { return compareInst(builder, L, leftType, tt, R, rightType.withoutOptions()); }); _lfb->setLastBlock(builder.GetInsertBlock()); return resVal; @@ -1154,18 +1148,15 @@ namespace tuplex { return listInclusionCheck(builder, L, leftType, R, rightType.withoutOptions()); } else { // exception check left + if (L_isnull) { + _lfb->addException(builder, ExceptionCode::TYPEERROR, L_isnull); + } - if(tt != TokenType::IS && tt != TokenType::ISNOT) { - if (L_isnull) { - _lfb->addException(builder, ExceptionCode::TYPEERROR, L_isnull); - } - - // exception check right - if (R_isnull) { - _lfb->addException(builder, ExceptionCode::TYPEERROR, R_isnull); - } + // exception check right + if (R_isnull) { + _lfb->addException(builder, ExceptionCode::TYPEERROR, R_isnull); } - + auto resVal = compareInst(builder, L, leftType.withoutOptions(), tt, R, rightType.withoutOptions()); _lfb->setLastBlock(builder.GetInsertBlock()); return resVal; diff --git a/tuplex/python/tests/test_is.py b/tuplex/python/tests/test_is.py index 1e7aa5b18..16ca30b97 100644 --- a/tuplex/python/tests/test_is.py +++ b/tuplex/python/tests/test_is.py @@ -27,7 +27,3 @@ def test_mixedIsNotNone(self): def test_mixedIsNotNone2(self): res = self.c.parallelize([None, True, False]).map(lambda x: x is not None).collect() self.assertEqual(res, [False, True, True]) - - # def test_throwError(self): - # res = self.c.parallelize(["hi", 0.5, {'a': 0}]).map(lambda x: x is not True).collect() - # self.assertRaises(TypeError) diff --git a/tuplex/test/core/IsKeywordTest.cc b/tuplex/test/core/IsKeywordTest.cc index 19769c80f..e57627d92 100644 --- a/tuplex/test/core/IsKeywordTest.cc +++ b/tuplex/test/core/IsKeywordTest.cc @@ -147,26 +147,26 @@ TEST_F(IsKeywordTest, NoneIsNotNone) { } } +// this test is invalid because codegen will only work for one of the types. +// TEST_F(IsKeywordTest, MixedIsNotNone) { +// using namespace tuplex; -TEST_F(IsKeywordTest, MixedIsNotNone) { - using namespace tuplex; - - Context c(microTestOptions()); - Row row1(Field::null()); - Row row2(Field(true)); - Row row3(Field::null()); - Row row4(Field(false)); +// Context c(microTestOptions()); +// Row row1(Field::null()); +// Row row2(Field(true)); +// Row row3(Field::null()); +// Row row4(Field(false)); - auto code = "lambda x: x is not None"; - auto m = c.parallelize({row1, row2, row3, row4}) - .map(UDF(code)).collectAsVector(); +// auto code = "lambda x: x is not None"; +// auto m = c.parallelize({row1, row2, row3, row4}) +// .map(UDF(code)).collectAsVector(); - EXPECT_EQ(m.size(), 4); - assert(m[0].toPythonString() == "(False,)"); - assert(m[1].toPythonString() == "(True,)"); - assert(m[2].toPythonString() == "(False,)"); - assert(m[3].toPythonString() == "(True,)"); -} +// EXPECT_EQ(m.size(), 4); +// assert(m[0].toPythonString() == "(False,)"); +// assert(m[1].toPythonString() == "(True,)"); +// assert(m[2].toPythonString() == "(False,)"); +// assert(m[3].toPythonString() == "(True,)"); +// } /* From 4ffd04c686d34bdf8783308fa7308ebdefca53d3 Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Fri, 22 Oct 2021 08:04:13 -0400 Subject: [PATCH 05/14] nit --- tuplex/python/tests/test_is.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tuplex/python/tests/test_is.py b/tuplex/python/tests/test_is.py index 16ca30b97..18e8db349 100644 --- a/tuplex/python/tests/test_is.py +++ b/tuplex/python/tests/test_is.py @@ -18,12 +18,10 @@ def test_boolIsNone(self): res = self.c.parallelize([True, False, False, True]).map(lambda x: x is None).collect() self.assertEqual(res, [False] * 4) - # cannot put None here, because None becomes def test_mixedIsNotNone(self): res = self.c.parallelize([None, None, None]).map(lambda x: x is not None).collect() self.assertEqual(res, [False, False, False]) - # this test fails for none, probably has to do with normal case. def test_mixedIsNotNone2(self): res = self.c.parallelize([None, True, False]).map(lambda x: x is not None).collect() self.assertEqual(res, [False, True, True]) From 5ee94443bdfa26d0536f4134c4db216014eaab77 Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Sat, 23 Oct 2021 14:06:02 -0400 Subject: [PATCH 06/14] misunderstood domain for is --- tuplex/codegen/include/IFailable.h | 1 + tuplex/codegen/src/BlockGeneratorVisitor.cc | 16 ++- tuplex/core/src/TraceVisitor.cc | 11 +- tuplex/python/tests/test_is.py | 7 +- tuplex/test/core/IsKeywordTest.cc | 145 ++++---------------- tuplex/test/core/PythonTracer.cc | 10 ++ 6 files changed, 63 insertions(+), 127 deletions(-) diff --git a/tuplex/codegen/include/IFailable.h b/tuplex/codegen/include/IFailable.h index 9280f9a20..83ce0bf0e 100644 --- a/tuplex/codegen/include/IFailable.h +++ b/tuplex/codegen/include/IFailable.h @@ -30,6 +30,7 @@ enum class CompileError { TYPE_ERROR_RETURN_ITERATOR, TYPE_ERROR_NEXT_CALL_DIFFERENT_DEFAULT_TYPE, TYPE_ERROR_MIXED_ASTNODETYPE_IN_FOR_LOOP_EXPRLIST, // exprlist contains a mix of tuple/list of identifiers and single identifier + TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON, }; /*! diff --git a/tuplex/codegen/src/BlockGeneratorVisitor.cc b/tuplex/codegen/src/BlockGeneratorVisitor.cc index 09a6daf7f..3f6524562 100644 --- a/tuplex/codegen/src/BlockGeneratorVisitor.cc +++ b/tuplex/codegen/src/BlockGeneratorVisitor.cc @@ -961,10 +961,10 @@ namespace tuplex { assert(R); if(tt == TokenType::IS || tt == TokenType::ISNOT) { + assert(leftType == python::Type::BOOLEAN && rightType == python::Type::BOOLEAN); // type must be boolean, otherwise compareInst with _isnull would've taken care. - bool invertResult = (tt == TokenType::ISNOT); - return invertResult ? _env->upcastToBoolean(builder, builder.CreateICmp(llvm::CmpInst::Predicate::ICMP_NE, L, R)) - : _env->upcastToBoolean(builder, builder.CreateICmp(llvm::CmpInst::Predicate::ICMP_EQ, L, R)); + auto cmpInst = (tt == TokenType::ISNOT) ? llvm::CmpInst::Predicate::ICMP_NE : llvm::CmpInst::Predicate::ICMP_EQ; + return _env->upcastToBoolean(builder, builder.CreateICmp(cmpInst, L, R)); } // comparison of values without null @@ -989,8 +989,9 @@ namespace tuplex { llvm::Value* BlockGeneratorVisitor::oneSidedNullComparison(llvm::IRBuilder<>& builder, const python::Type& type, const TokenType& tt, llvm::Value* isnull) { - assert(tt == TokenType::EQEQUAL || tt == TokenType::NOTEQUAL || tt == TokenType::IS || tt == TokenType::ISNOT); // only for == or !=! + assert(tt == TokenType::EQEQUAL || tt == TokenType::NOTEQUAL || tt == TokenType::IS || tt == TokenType::ISNOT); // only for == or != or IS or ISNOT! + // we're comparing null to null, should only return true if operators are EQEQUAL or IS. if(type == python::Type::NULLVALUE) return _env->boolConst(tt == TokenType::EQEQUAL || tt == TokenType::IS); // if == then true, if != then false @@ -1004,6 +1005,10 @@ namespace tuplex { // the other side is null // if isnull is true && equal => true // if isnull is false && notequal => false (case 12 != None) + + // for IS NOT, if isnull is true, we want to return false. + // if isnull is false, we want to return true. + // therefore we negate. (similar to logic for NOTEQUAL). if(tt == TokenType::NOTEQUAL || tt == TokenType::ISNOT) return _env->upcastToBoolean(builder, _env->i1neg(builder, isnull)); else @@ -1012,6 +1017,9 @@ namespace tuplex { // the other side is null // => 12 != null => true // => 12 == null => false + + // we are now comparing a non-null type to null. + // so we return true only if token is IS NOT or NOTEQUAL. return _env->boolConst(tt == TokenType::NOTEQUAL || tt == TokenType::ISNOT); } } diff --git a/tuplex/core/src/TraceVisitor.cc b/tuplex/core/src/TraceVisitor.cc index 92854453b..9e7c7665e 100644 --- a/tuplex/core/src/TraceVisitor.cc +++ b/tuplex/core/src/TraceVisitor.cc @@ -388,8 +388,7 @@ namespace tuplex { // now truth value testing, single element? auto res = ti_vals.front(); - // IS and IS NOT were added here with these op ids because as far as optimizations go, - // their functionality should be similar to EQ/NE. + // IS and IS NOT are equivalent to id(L) == id(R) and id(L) != id(R). std::unordered_map cmpLookup{{TokenType::EQEQUAL, Py_EQ}, {TokenType::IS, Py_EQ}, {TokenType::ISNOT, Py_NE}, @@ -411,8 +410,12 @@ namespace tuplex { auto info = python::PyString_AsString(res.value) + " " + opToString(op) + " " + python::PyString_AsString(ti_vals[i+1].value); - // cpython api. - res.value = PyObject_RichCompare(res.value, ti_vals[i + 1].value, opid); + if(op == TokenType::IS || op == TokenType::ISNOT) { + // compare pointers in case of `is` keyword. + res.value = (res.value == ti_vals[i + 1].value) ? Py_True : Py_False; + } else { + res.value = PyObject_RichCompare(res.value, ti_vals[i + 1].value, opid); + } auto res_info = "is: " + python::PyString_AsString(res.value); diff --git a/tuplex/python/tests/test_is.py b/tuplex/python/tests/test_is.py index 18e8db349..ddc94e726 100644 --- a/tuplex/python/tests/test_is.py +++ b/tuplex/python/tests/test_is.py @@ -1,9 +1,12 @@ -from unittest import TestCase import tuplex +from unittest import TestCase +""" +Tests +""" class TestIs(TestCase): def setUp(self): - self.conf = {"webui.enable": False, "driverMemory": "64MB", "executorMemory": "2MB", "partitionSize": "128KB"} + self.conf = {"webui.enable": False, "executorCount": "0"} self.c = tuplex.Context(webui=False) def test_boolIsBool(self): diff --git a/tuplex/test/core/IsKeywordTest.cc b/tuplex/test/core/IsKeywordTest.cc index e57627d92..6796117f9 100644 --- a/tuplex/test/core/IsKeywordTest.cc +++ b/tuplex/test/core/IsKeywordTest.cc @@ -11,59 +11,41 @@ class IsKeywordTest : public PyTest {}; -// bool saveToPDFx(tuplex::ASTNode* root, const std::string& path) { -// using namespace tuplex; -// assert(root); -// GraphVizGraph graph; -// graph.createFromAST(root); -// return graph.saveAsPDF(path); -// } - -// // works -// TEST_F(IsKeywordTest, xIsNone) { -// using namespace tuplex; -// auto code = "lambda x: x is None"; -// auto node = std::unique_ptr(tuplex::parseToAST(code)); -// ASSERT_TRUE(node); -// printParseTree(code, std::cout); -// saveToPDFx(node.get(), "xIsNone.pdf"); -// } - -TEST_F(IsKeywordTest, OptionBoolIsBool) { +TEST_F(IsKeywordTest, OptionIsBool) { using namespace tuplex; Context c(microTestOptions()); Row row1(Field(option(true))); - Row row2(Field(option(true))); - Row row3(Field(option(true))); - Row row4(Field(option(true))); + Row row2(Field(option::none)); + Row row3(Field(option(false))); auto code = "lambda x: x is True"; - auto m = c.parallelize({row1, row2, row3, row4}) + auto m = c.parallelize({row1, row2, row3}) .map(UDF(code)).collectAsVector(); EXPECT_EQ(m.size(), 4); - for(int i = 0; i < m.size(); i++) { - assert(m[i].toPythonString() == "(True,)"); + assert(m[0].toPythonString() == "(True,)"); + for(int i = 1; i < m.size(); i++) { + assert(m[i].toPythonString() == "(False,)"); } } -TEST_F(IsKeywordTest, OptionBoolIsNotBool) { +TEST_F(IsKeywordTest, OptionIsNotBool) { using namespace tuplex; Context c(microTestOptions()); Row row1(Field(option(true))); - Row row2(Field(option(true))); - Row row3(Field(option(true))); - Row row4(Field(option(true))); + Row row2(Field(option(false))); + Row row3(Field(option::none)); auto code = "lambda x: x is not True"; - auto m = c.parallelize({row1, row2, row3, row4}) + auto m = c.parallelize({row1, row2, row3}) .map(UDF(code)).collectAsVector(); EXPECT_EQ(m.size(), 4); - for(int i = 0; i < m.size(); i++) { - assert(m[i].toPythonString() == "(False,)"); + assert(m[0].toPythonString() == "(False,)"); + for(int i = 1; i < m.size(); i++) { + assert(m[i].toPythonString() == "(True,)"); } } @@ -75,7 +57,7 @@ TEST_F(IsKeywordTest, BoolIsBool) { Row row1(Field(true)); Row row2(Field(false)); Row row3(Field(false)); - Row row4(Field(true)); + Row row4(Field(false)); auto code = "lambda x: x is True"; auto m = c.parallelize({row1, row2, row3, row4}) @@ -85,7 +67,7 @@ TEST_F(IsKeywordTest, BoolIsBool) { assert(m[0].toPythonString() == "(True,)"); assert(m[1].toPythonString() == "(False,)"); assert(m[2].toPythonString() == "(False,)"); - assert(m[3].toPythonString() == "(True,)"); + assert(m[3].toPythonString() == "(False,)"); } TEST_F(IsKeywordTest, BoolIsNotBool) { @@ -113,19 +95,15 @@ TEST_F(IsKeywordTest, NoneIsNone) { using namespace tuplex; Context c(microTestOptions()); - Row row1(Field::null()); - Row row2(Field::null()); - Row row3(Field::null()); - Row row4(Field::null()); + Row row1(Field(option(10))); + Row row2(Field(option::none)); auto code = "lambda x: x is None"; - auto m = c.parallelize({row1, row2, row3, row4}) + auto m = c.parallelize({row1, row2}) .map(UDF(code)).collectAsVector(); - EXPECT_EQ(m.size(), 4); - for(int i = 0; i < m.size(); i++) { - assert(m[i].toPythonString() == "(True,)"); - } + assert(m[0].toPythonString() == "(False,)"); + assert(m[1].toPythonString() == "(True,)"); } TEST_F(IsKeywordTest, NoneIsNotNone) { @@ -133,8 +111,8 @@ TEST_F(IsKeywordTest, NoneIsNotNone) { Context c(microTestOptions()); Row row1(Field::null()); - Row row2(Field::null()); - Row row3(Field::null()); + Row row2(Field(option(42))); + Row row3(Field(false)); Row row4(Field::null()); auto code = "lambda x: x is not None"; @@ -142,78 +120,11 @@ TEST_F(IsKeywordTest, NoneIsNotNone) { .map(UDF(code)).collectAsVector(); EXPECT_EQ(m.size(), 4); - for(int i = 0; i < m.size(); i++) { - assert(m[i].toPythonString() == "(False,)"); + + assert(m[0].toPythonString() == "(False,)"); + for(int i = 1; i < m.size() - 1; i++) { + assert(m[i].toPythonString() == "(True,)"); } + assert(m[3].toPythonString() == "(False,)"); } -// this test is invalid because codegen will only work for one of the types. -// TEST_F(IsKeywordTest, MixedIsNotNone) { -// using namespace tuplex; - -// Context c(microTestOptions()); -// Row row1(Field::null()); -// Row row2(Field(true)); -// Row row3(Field::null()); -// Row row4(Field(false)); - -// auto code = "lambda x: x is not None"; -// auto m = c.parallelize({row1, row2, row3, row4}) -// .map(UDF(code)).collectAsVector(); - -// EXPECT_EQ(m.size(), 4); -// assert(m[0].toPythonString() == "(False,)"); -// assert(m[1].toPythonString() == "(True,)"); -// assert(m[2].toPythonString() == "(False,)"); -// assert(m[3].toPythonString() == "(True,)"); -// } - - -/* - Even though a std::runtime_error is thrown in BlockGeneratorVisitor.cc, it - doesn't get caught in catch(...) for some reason. -*/ - - -// TEST_F(IsKeywordTest, AnyIsNone) { -// using namespace tuplex; - -// // Logger::instance().defaultLogger().warn("asdgasdjlkgalksdgjalksdjgalksdjgas"); - -// Context c(microTestOptions()); -// Row row1(Field(option(5))); -// Row row2(Field("hi")); -// Row row3(Field(0.55)); -// Row row4(Field("0.55")); - -// auto code = "lambda x: x is None"; - -// try { -// auto udf = UDF(code); -// } catch(const std::exception& e) { -// std::string s = e.what(); -// Logger::instance().defaultLogger().error("exception string is this: " + s); -// } catch(...) { -// Logger::instance().defaultLogger().error("hmm, different exception occured"); -// } - -// auto m = c.parallelize({row1, row2, row3, row4}) -// .map(UDF(code)).collectAsVector(); - -// } - -// TEST_F(IsKeywordTest, AnyIsNotNone) { -// using namespace tuplex; - -// Context c(microTestOptions()); -// Row row1(Field(option(5))); -// Row row2(Field("hi")); -// Row row3(Field(0.55)); -// Row row4(Field("0.55")); - -// auto code = "lambda x: x is not None"; -// auto m = c.parallelize({row1, row2, row3, row4}) -// .map(UDF(code)).collectAsVector(); - -// EXPECT_EQ(m.size(), 0); -// } diff --git a/tuplex/test/core/PythonTracer.cc b/tuplex/test/core/PythonTracer.cc index 923ce829c..18c6e0b0b 100644 --- a/tuplex/test/core/PythonTracer.cc +++ b/tuplex/test/core/PythonTracer.cc @@ -161,6 +161,16 @@ TEST_F(TracerTest, IsKeyword) { traceAndValidateResult(udf4, arg4); + auto udf5 = "lambda x: x is 25"; + PyObject* arg5 = PyLong_FromLong(25); + + traceAndValidateResult(udf5, arg5); + + auto udf6 = "lambda x: x is 400"; + PyObject* arg6 = PyLong_FromLong(400); + + traceAndValidateResult(udf6, arg6); + python::unlockGIL(); } From 8b0eff9632eb97e70c048c32e6ebe267b9e9329d Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Sat, 23 Oct 2021 17:40:49 -0400 Subject: [PATCH 07/14] passing tests --- tuplex/codegen/src/BlockGeneratorVisitor.cc | 18 +++-- tuplex/codegen/src/IFailable.cc | 3 + tuplex/core/src/TraceVisitor.cc | 10 ++- tuplex/test/core/IsKeywordTest.cc | 76 ++++++++++++++++++--- tuplex/test/core/PythonTracer.cc | 16 +++++ 5 files changed, 103 insertions(+), 20 deletions(-) diff --git a/tuplex/codegen/src/BlockGeneratorVisitor.cc b/tuplex/codegen/src/BlockGeneratorVisitor.cc index 3f6524562..5a39271f6 100644 --- a/tuplex/codegen/src/BlockGeneratorVisitor.cc +++ b/tuplex/codegen/src/BlockGeneratorVisitor.cc @@ -961,8 +961,14 @@ namespace tuplex { assert(R); if(tt == TokenType::IS || tt == TokenType::ISNOT) { - assert(leftType == python::Type::BOOLEAN && rightType == python::Type::BOOLEAN); - // type must be boolean, otherwise compareInst with _isnull would've taken care. + assert(leftType == python::Type::BOOLEAN || rightType == python::Type::BOOLEAN); + // one of the types must be boolean, otherwise compareInst with _isnull would've taken care. + if((leftType == python::Type::BOOLEAN) ^ (rightType == python::Type::BOOLEAN)) { + // one of the types is boolean, other isn't. comparison results in false. + return _env->boolConst(tt == TokenType::ISNOT); + } + + // both must be boolean. auto cmpInst = (tt == TokenType::ISNOT) ? llvm::CmpInst::Predicate::ICMP_NE : llvm::CmpInst::Predicate::ICMP_EQ; return _env->upcastToBoolean(builder, builder.CreateICmp(cmpInst, L, R)); } @@ -1031,13 +1037,12 @@ namespace tuplex { // None comparisons only work for == or !=, i.e. for all other ops throw exception if (tt == TokenType::EQEQUAL || tt == TokenType::NOTEQUAL || tt == TokenType::IS || tt == TokenType::ISNOT) { - // special case: one side is None if(tt == TokenType::IS || tt == TokenType::ISNOT) { std::unordered_set validTypes = {python::Type::BOOLEAN, python::Type::NULLVALUE}; - if (!(validTypes.count(leftType.withoutOptions()) && validTypes.count(rightType.withoutOptions()))) { + if (!(validTypes.count(leftType.withoutOptions()) || validTypes.count(rightType.withoutOptions()))) { std::stringstream ss; - ss << "Cannot generate is comparison for types " + ss << "Could not generate comparison for types " << leftType.desc() << " " << opToString(tt) << " " << rightType.desc(); @@ -1047,6 +1052,7 @@ namespace tuplex { } } + // special case: one side is None if(leftType == python::Type::NULLVALUE || rightType == python::Type::NULLVALUE) { // left side NULL VALUE? @@ -1107,7 +1113,7 @@ namespace tuplex { // compareInst if both are NOT none auto bothValid = builder.CreateAnd(L_isnull, R_isnull); auto xorResult = builder.CreateXor(L_isnull, R_isnull); - if (TokenType::EQEQUAL == tt) + if (tt == TokenType::EQEQUAL || tt == TokenType::IS) xorResult = builder.CreateNot(xorResult); auto resVal = _env->CreateTernaryLogic(builder, bothValid, [&] (llvm::IRBuilder<>& builder) { return compareInst(builder, L, leftType.withoutOptions(), tt, R, diff --git a/tuplex/codegen/src/IFailable.cc b/tuplex/codegen/src/IFailable.cc index c141efa04..08e9c0149 100644 --- a/tuplex/codegen/src/IFailable.cc +++ b/tuplex/codegen/src/IFailable.cc @@ -71,6 +71,9 @@ std::string IFailable::compileErrorToStr(const CompileError &err) { case CompileError::TYPE_ERROR_MIXED_ASTNODETYPE_IN_FOR_LOOP_EXPRLIST: errMsg = "mixed use of tuple/list of identifiers and single identifier in exprlist not yet supported"; break; + case CompileError::TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON: + errMsg = "use of is comparison only supported with types boolean and null"; + break; default: break; } diff --git a/tuplex/core/src/TraceVisitor.cc b/tuplex/core/src/TraceVisitor.cc index 9e7c7665e..4f6370b8d 100644 --- a/tuplex/core/src/TraceVisitor.cc +++ b/tuplex/core/src/TraceVisitor.cc @@ -410,15 +410,19 @@ namespace tuplex { auto info = python::PyString_AsString(res.value) + " " + opToString(op) + " " + python::PyString_AsString(ti_vals[i+1].value); + // based on op, decide value of result. + if(op == TokenType::IS || op == TokenType::ISNOT) { - // compare pointers in case of `is` keyword. - res.value = (res.value == ti_vals[i + 1].value) ? Py_True : Py_False; + bool finalResult = (ti_vals[i + 1].value == res.value); + // invert result if op is ISNOT. + finalResult = (op == TokenType::IS) ? finalResult : !finalResult; + res.value = finalResult ? Py_True : Py_False; } else { res.value = PyObject_RichCompare(res.value, ti_vals[i + 1].value, opid); } + // res.value = PyObject_RichCompare(res.value, ti_vals[i + 1].value, opid); auto res_info = "is: " + python::PyString_AsString(res.value); - // NULL? ==> failure! assert(res.value); } diff --git a/tuplex/test/core/IsKeywordTest.cc b/tuplex/test/core/IsKeywordTest.cc index 6796117f9..3c95ccdae 100644 --- a/tuplex/test/core/IsKeywordTest.cc +++ b/tuplex/test/core/IsKeywordTest.cc @@ -11,6 +11,10 @@ class IsKeywordTest : public PyTest {}; +/* + Tests for codegen support for `is` keyword (only for True, False and None types). +*/ + TEST_F(IsKeywordTest, OptionIsBool) { using namespace tuplex; @@ -23,7 +27,7 @@ TEST_F(IsKeywordTest, OptionIsBool) { auto m = c.parallelize({row1, row2, row3}) .map(UDF(code)).collectAsVector(); - EXPECT_EQ(m.size(), 4); + EXPECT_EQ(m.size(), 3); assert(m[0].toPythonString() == "(True,)"); for(int i = 1; i < m.size(); i++) { assert(m[i].toPythonString() == "(False,)"); @@ -42,7 +46,7 @@ TEST_F(IsKeywordTest, OptionIsNotBool) { auto m = c.parallelize({row1, row2, row3}) .map(UDF(code)).collectAsVector(); - EXPECT_EQ(m.size(), 4); + EXPECT_EQ(m.size(), 3); assert(m[0].toPythonString() == "(False,)"); for(int i = 1; i < m.size(); i++) { assert(m[i].toPythonString() == "(True,)"); @@ -102,6 +106,7 @@ TEST_F(IsKeywordTest, NoneIsNone) { auto m = c.parallelize({row1, row2}) .map(UDF(code)).collectAsVector(); + EXPECT_EQ(m.size(), 2); assert(m[0].toPythonString() == "(False,)"); assert(m[1].toPythonString() == "(True,)"); } @@ -110,21 +115,70 @@ TEST_F(IsKeywordTest, NoneIsNotNone) { using namespace tuplex; Context c(microTestOptions()); - Row row1(Field::null()); - Row row2(Field(option(42))); - Row row3(Field(false)); + Row row1(Field(option(-1))); + Row row2(Field(option(45))); + Row row3(Field(option::none)); Row row4(Field::null()); auto code = "lambda x: x is not None"; - auto m = c.parallelize({row1, row2, row3, row4}) + auto m = c.parallelize({row1, row2, row3}) .map(UDF(code)).collectAsVector(); - EXPECT_EQ(m.size(), 4); + EXPECT_EQ(m.size(), 3); + + assert(m[0].toPythonString() == "(True,)"); + assert(m[1].toPythonString() == "(True,)"); + assert(m[2].toPythonString() == "(False,)"); +} + +TEST_F(IsKeywordTest, StringIsNotNone) { + using namespace tuplex; + + Context c(microTestOptions()); + Row row1(Field(option("hello"))); + Row row2(Field(option::none)); + + auto code = "lambda x: x is not None"; + auto m = c.parallelize({row1, row2}) + .map(UDF(code)).collectAsVector(); + + EXPECT_EQ(m.size(), 2); + + assert(m[0].toPythonString() == "(True,)"); + assert(m[1].toPythonString() == "(False,)"); +} + +TEST_F(IsKeywordTest, StringIsBool) { + using namespace tuplex; + + Context c(microTestOptions()); + Row row1(Field(option("hello"))); + Row row2(Field(option::none)); + + auto code = "lambda x: x is True"; + auto m = c.parallelize({row1, row2}) + .map(UDF(code)).collectAsVector(); + + EXPECT_EQ(m.size(), 2); assert(m[0].toPythonString() == "(False,)"); - for(int i = 1; i < m.size() - 1; i++) { - assert(m[i].toPythonString() == "(True,)"); - } - assert(m[3].toPythonString() == "(False,)"); + assert(m[1].toPythonString() == "(False,)"); } +TEST_F(IsKeywordTest, FailingTest) { + using namespace tuplex; + + Context c(microTestOptions()); + + Row row1(45); + Row row2(false); + Row row3("hi"); + + auto code = "lambda x: x is \"hi\""; + + auto node = UDF(code); + + auto m = c.parallelize({row1, row2, row3}) + .map(node).collectAsVector(); + +} diff --git a/tuplex/test/core/PythonTracer.cc b/tuplex/test/core/PythonTracer.cc index 18c6e0b0b..c0e0f756e 100644 --- a/tuplex/test/core/PythonTracer.cc +++ b/tuplex/test/core/PythonTracer.cc @@ -171,6 +171,22 @@ TEST_F(TracerTest, IsKeyword) { traceAndValidateResult(udf6, arg6); + auto udf7 = "lambda x: x is 1"; + PyObject* arg7 = Py_None; + + traceAndValidateResult(udf7, arg7); + + auto udf8 = "lambda x: x is not 400"; + PyObject* arg8 = PyBool_FromLong(0); + + traceAndValidateResult(udf8, arg8); + + auto udf9 = "lambda x: x is 0"; + PyObject* arg9 = PyBool_FromLong(0); + + traceAndValidateResult(udf9, arg9); + + python::unlockGIL(); } From 66845b284dba2a3cd49760dd099811ed3730fa15 Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Tue, 26 Oct 2021 16:29:09 -0400 Subject: [PATCH 08/14] address comments --- tuplex/codegen/include/IFailable.h | 2 +- tuplex/codegen/src/BlockGeneratorVisitor.cc | 20 +++----------------- tuplex/codegen/src/TypeAnnotatorVisitor.cc | 21 +++++++++++++++++++++ tuplex/core/src/TraceVisitor.cc | 3 ++- tuplex/python/tests/test_is.py | 12 ++++++++++-- tuplex/test/core/IsKeywordTest.cc | 5 +++++ 6 files changed, 42 insertions(+), 21 deletions(-) diff --git a/tuplex/codegen/include/IFailable.h b/tuplex/codegen/include/IFailable.h index 83ce0bf0e..761c458a9 100644 --- a/tuplex/codegen/include/IFailable.h +++ b/tuplex/codegen/include/IFailable.h @@ -30,7 +30,7 @@ enum class CompileError { TYPE_ERROR_RETURN_ITERATOR, TYPE_ERROR_NEXT_CALL_DIFFERENT_DEFAULT_TYPE, TYPE_ERROR_MIXED_ASTNODETYPE_IN_FOR_LOOP_EXPRLIST, // exprlist contains a mix of tuple/list of identifiers and single identifier - TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON, + TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON, // incompatible types for `is` comparison (one of the types is not BOOLEAN/NULLVALUE). }; /*! diff --git a/tuplex/codegen/src/BlockGeneratorVisitor.cc b/tuplex/codegen/src/BlockGeneratorVisitor.cc index 5a39271f6..a9e35d8e3 100644 --- a/tuplex/codegen/src/BlockGeneratorVisitor.cc +++ b/tuplex/codegen/src/BlockGeneratorVisitor.cc @@ -963,14 +963,14 @@ namespace tuplex { if(tt == TokenType::IS || tt == TokenType::ISNOT) { assert(leftType == python::Type::BOOLEAN || rightType == python::Type::BOOLEAN); // one of the types must be boolean, otherwise compareInst with _isnull would've taken care. - if((leftType == python::Type::BOOLEAN) ^ (rightType == python::Type::BOOLEAN)) { + if((leftType == python::Type::BOOLEAN) != (rightType == python::Type::BOOLEAN)) { // one of the types is boolean, other isn't. comparison results in false. return _env->boolConst(tt == TokenType::ISNOT); } // both must be boolean. - auto cmpInst = (tt == TokenType::ISNOT) ? llvm::CmpInst::Predicate::ICMP_NE : llvm::CmpInst::Predicate::ICMP_EQ; - return _env->upcastToBoolean(builder, builder.CreateICmp(cmpInst, L, R)); + auto cmpPredicate = (tt == TokenType::ISNOT) ? llvm::CmpInst::Predicate::ICMP_NE : llvm::CmpInst::Predicate::ICMP_EQ; + return _env->upcastToBoolean(builder, builder.CreateICmp(cmpPredicate, L, R)); } // comparison of values without null @@ -1038,20 +1038,6 @@ namespace tuplex { // None comparisons only work for == or !=, i.e. for all other ops throw exception if (tt == TokenType::EQEQUAL || tt == TokenType::NOTEQUAL || tt == TokenType::IS || tt == TokenType::ISNOT) { - if(tt == TokenType::IS || tt == TokenType::ISNOT) { - std::unordered_set validTypes = {python::Type::BOOLEAN, python::Type::NULLVALUE}; - if (!(validTypes.count(leftType.withoutOptions()) || validTypes.count(rightType.withoutOptions()))) { - std::stringstream ss; - ss << "Could not generate comparison for types " - << leftType.desc() - << " " << opToString(tt) << " " - << rightType.desc(); - error(ss.str()); - // return TRUE as dummy constant to continue tracking process - return _env->boolConst(true); - } - } - // special case: one side is None if(leftType == python::Type::NULLVALUE || rightType == python::Type::NULLVALUE) { diff --git a/tuplex/codegen/src/TypeAnnotatorVisitor.cc b/tuplex/codegen/src/TypeAnnotatorVisitor.cc index 5b491da22..8d3ad8ca2 100644 --- a/tuplex/codegen/src/TypeAnnotatorVisitor.cc +++ b/tuplex/codegen/src/TypeAnnotatorVisitor.cc @@ -535,6 +535,27 @@ namespace tuplex { else // else it is a bool (b.c. it is a compare statement) cmp->setInferredType(python::Type::BOOLEAN); + + + // check if `is` comparison is valid + std::unordered_set validTypes = {python::Type::NULLVALUE, python::Type::BOOLEAN}; + // one of every two types must be in validTypes. + + if(cmp->_comps.size() == 1) { + if(!validTypes.count(cmp->_left->getInferredType()) && !validTypes.count(cmp->_comps[0]->getInferredType())) { + // invalid types for lhs and rhs + addCompileError(CompileError::TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON); + } + } + + for(int i = 0; i < cmp->_comps.size() - 1; i++) { + auto currType = cmp->_comps[i]->getInferredType(); + auto nextType = cmp->_comps[i+1]->getInferredType(); + if(!validTypes.count(currType) && !validTypes.count(nextType)) { + // neither type is valid for an is comparison. + addCompileError(CompileError::TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON); + } + } } python::Type TypeAnnotatorVisitor::binaryOpInference(ASTNode* left, const python::Type& a, diff --git a/tuplex/core/src/TraceVisitor.cc b/tuplex/core/src/TraceVisitor.cc index 4f6370b8d..7d9e4d70f 100644 --- a/tuplex/core/src/TraceVisitor.cc +++ b/tuplex/core/src/TraceVisitor.cc @@ -413,7 +413,8 @@ namespace tuplex { // based on op, decide value of result. if(op == TokenType::IS || op == TokenType::ISNOT) { - bool finalResult = (ti_vals[i + 1].value == res.value); + assert(i+1 < ti_vals.size()); + bool finalResult = (ti_vals[i+1].value == res.value); // invert result if op is ISNOT. finalResult = (op == TokenType::IS) ? finalResult : !finalResult; res.value = finalResult ? Py_True : Py_False; diff --git a/tuplex/python/tests/test_is.py b/tuplex/python/tests/test_is.py index ddc94e726..7768d389a 100644 --- a/tuplex/python/tests/test_is.py +++ b/tuplex/python/tests/test_is.py @@ -1,13 +1,13 @@ import tuplex from unittest import TestCase """ -Tests +Tests functionality for `is` keyword. """ class TestIs(TestCase): def setUp(self): self.conf = {"webui.enable": False, "executorCount": "0"} - self.c = tuplex.Context(webui=False) + self.c = tuplex.Context(self.conf) def test_boolIsBool(self): res = self.c.parallelize([False, True, False, False, True]).map(lambda x: x is False).collect() @@ -21,6 +21,10 @@ def test_boolIsNone(self): res = self.c.parallelize([True, False, False, True]).map(lambda x: x is None).collect() self.assertEqual(res, [False] * 4) + def test_mixedIsNone(self): + res = self.c.parallelize([None, 255, 400, False, 2.3]).map(lambda x: x is None).collect() + self.assertEqual(res, [True, False, False, False, False]) + def test_mixedIsNotNone(self): res = self.c.parallelize([None, None, None]).map(lambda x: x is not None).collect() self.assertEqual(res, [False, False, False]) @@ -28,3 +32,7 @@ def test_mixedIsNotNone(self): def test_mixedIsNotNone2(self): res = self.c.parallelize([None, True, False]).map(lambda x: x is not None).collect() self.assertEqual(res, [False, True, True]) + + def test_mixedIsNotNone3(self): + res = self.c.parallelize([2, False, None]).map(lambda x: x is not None).collect() + self.assertEqual(res, [True, True, False]) diff --git a/tuplex/test/core/IsKeywordTest.cc b/tuplex/test/core/IsKeywordTest.cc index 3c95ccdae..070c4e8c3 100644 --- a/tuplex/test/core/IsKeywordTest.cc +++ b/tuplex/test/core/IsKeywordTest.cc @@ -170,6 +170,9 @@ TEST_F(IsKeywordTest, FailingTest) { Context c(microTestOptions()); + // reset log + logStream.str(""); + Row row1(45); Row row2(false); Row row3("hi"); @@ -181,4 +184,6 @@ TEST_F(IsKeywordTest, FailingTest) { auto m = c.parallelize({row1, row2, row3}) .map(node).collectAsVector(); + auto log = logStream.str(); + EXPECT_NE(log.find("use of is comparison only supported with types"), std::string::npos); } From 9437e892245cd59f5d4c9ed9ecb9a85840845701 Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Wed, 27 Oct 2021 07:42:05 -0400 Subject: [PATCH 09/14] should pass tests now --- tuplex/codegen/include/ASTNodes.h | 4 ++-- tuplex/codegen/src/TypeAnnotatorVisitor.cc | 14 +++++++++---- tuplex/test/core/IsKeywordTest.cc | 23 ++++++++++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/tuplex/codegen/include/ASTNodes.h b/tuplex/codegen/include/ASTNodes.h index d9e34e14b..039a00355 100644 --- a/tuplex/codegen/include/ASTNodes.h +++ b/tuplex/codegen/include/ASTNodes.h @@ -1043,8 +1043,8 @@ namespace tuplex { public: ASTNode *_left; - std::vector _ops; // operands - std::vector _comps; // comparators + std::vector _ops; // operators (TokenType::IS, TokenType::NOTEQUAL, etc.) + std::vector _comps; // operands (54, "hello", False, etc.) NCompare() : _left(nullptr) {} diff --git a/tuplex/codegen/src/TypeAnnotatorVisitor.cc b/tuplex/codegen/src/TypeAnnotatorVisitor.cc index 8d3ad8ca2..17ae955ce 100644 --- a/tuplex/codegen/src/TypeAnnotatorVisitor.cc +++ b/tuplex/codegen/src/TypeAnnotatorVisitor.cc @@ -541,19 +541,25 @@ namespace tuplex { std::unordered_set validTypes = {python::Type::NULLVALUE, python::Type::BOOLEAN}; // one of every two types must be in validTypes. - if(cmp->_comps.size() == 1) { - if(!validTypes.count(cmp->_left->getInferredType()) && !validTypes.count(cmp->_comps[0]->getInferredType())) { - // invalid types for lhs and rhs + if(cmp->_comps.size() >= 1) { + if(cmp->_ops[0] == TokenType::IS && !validTypes.count(cmp->_left->getInferredType()) && !validTypes.count(cmp->_comps[0]->getInferredType())) { + // invalid types for lhs and rhs to do an `is` comparison. addCompileError(CompileError::TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON); + return; } } + bool lastValid = false; + // if more that one operand, check if all types would be valid. for(int i = 0; i < cmp->_comps.size() - 1; i++) { auto currType = cmp->_comps[i]->getInferredType(); auto nextType = cmp->_comps[i+1]->getInferredType(); - if(!validTypes.count(currType) && !validTypes.count(nextType)) { + if(!validTypes.count(currType) && !validTypes.count(nextType) && cmp->_ops[i] == TokenType::IS && !lastValid) { // neither type is valid for an is comparison. addCompileError(CompileError::TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON); + return; + } else { + lastValid = true; } } } diff --git a/tuplex/test/core/IsKeywordTest.cc b/tuplex/test/core/IsKeywordTest.cc index 070c4e8c3..2ebf3ded6 100644 --- a/tuplex/test/core/IsKeywordTest.cc +++ b/tuplex/test/core/IsKeywordTest.cc @@ -187,3 +187,26 @@ TEST_F(IsKeywordTest, FailingTest) { auto log = logStream.str(); EXPECT_NE(log.find("use of is comparison only supported with types"), std::string::npos); } + +TEST_F(IsKeywordTest, MultipleIs) { + using namespace tuplex; + + Context c(microTestOptions()); + + // reset log + logStream.str(""); + + Row row1(45); + Row row2(false); + Row row3("hi"); + + auto code = "lambda x: x is \"hi\" != False"; + + auto node = UDF(code); + + auto m = c.parallelize({row1, row2, row3}) + .map(node).collectAsVector(); + + auto log = logStream.str(); + EXPECT_NE(log.find("use of is comparison only supported with types"), std::string::npos); +} From 06383417a99c4e8743f11bff8c4c7eff6ee295cb Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Wed, 27 Oct 2021 07:47:29 -0400 Subject: [PATCH 10/14] nit --- tuplex/codegen/src/TypeAnnotatorVisitor.cc | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tuplex/codegen/src/TypeAnnotatorVisitor.cc b/tuplex/codegen/src/TypeAnnotatorVisitor.cc index 17ae955ce..e98a92563 100644 --- a/tuplex/codegen/src/TypeAnnotatorVisitor.cc +++ b/tuplex/codegen/src/TypeAnnotatorVisitor.cc @@ -554,10 +554,18 @@ namespace tuplex { for(int i = 0; i < cmp->_comps.size() - 1; i++) { auto currType = cmp->_comps[i]->getInferredType(); auto nextType = cmp->_comps[i+1]->getInferredType(); - if(!validTypes.count(currType) && !validTypes.count(nextType) && cmp->_ops[i] == TokenType::IS && !lastValid) { + + // type error only if previous comparison is invalid + if(!validTypes.count(currType) && !validTypes.count(nextType) && cmp->_ops[i] == TokenType::IS) { // neither type is valid for an is comparison. - addCompileError(CompileError::TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON); - return; + if(!lastValid) { + // previous was invalid, current is also invalid, therefore incompatible types. + addCompileError(CompileError::TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON); + return; + } else { + // the one before this was a valid comparison, resulting in a bool, so can't error yet. + lastValid = false; + } } else { lastValid = true; } From d2aab593eb2381976edec24dbd157002dc0c7d48 Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Thu, 28 Oct 2021 10:46:17 -0400 Subject: [PATCH 11/14] add associativity tests, modify is typecheck logic, add file header --- tuplex/codegen/src/TypeAnnotatorVisitor.cc | 11 +-- tuplex/core/src/TraceVisitor.cc | 6 +- tuplex/test/core/IsKeywordTest.cc | 81 +++++++++++++++++++++- 3 files changed, 84 insertions(+), 14 deletions(-) diff --git a/tuplex/codegen/src/TypeAnnotatorVisitor.cc b/tuplex/codegen/src/TypeAnnotatorVisitor.cc index e98a92563..276afefbc 100644 --- a/tuplex/codegen/src/TypeAnnotatorVisitor.cc +++ b/tuplex/codegen/src/TypeAnnotatorVisitor.cc @@ -558,16 +558,7 @@ namespace tuplex { // type error only if previous comparison is invalid if(!validTypes.count(currType) && !validTypes.count(nextType) && cmp->_ops[i] == TokenType::IS) { // neither type is valid for an is comparison. - if(!lastValid) { - // previous was invalid, current is also invalid, therefore incompatible types. - addCompileError(CompileError::TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON); - return; - } else { - // the one before this was a valid comparison, resulting in a bool, so can't error yet. - lastValid = false; - } - } else { - lastValid = true; + addCompileError(CompileError::TYPE_ERROR_INCOMPATIBLE_TYPES_FOR_IS_COMPARISON); } } } diff --git a/tuplex/core/src/TraceVisitor.cc b/tuplex/core/src/TraceVisitor.cc index 7d9e4d70f..e5056148d 100644 --- a/tuplex/core/src/TraceVisitor.cc +++ b/tuplex/core/src/TraceVisitor.cc @@ -390,8 +390,6 @@ namespace tuplex { // IS and IS NOT are equivalent to id(L) == id(R) and id(L) != id(R). std::unordered_map cmpLookup{{TokenType::EQEQUAL, Py_EQ}, - {TokenType::IS, Py_EQ}, - {TokenType::ISNOT, Py_NE}, {TokenType::NOTEQUAL, Py_NE}, {TokenType::LESS, Py_LT}, {TokenType::LESSEQUAL, Py_LE}, @@ -411,8 +409,10 @@ namespace tuplex { python::PyString_AsString(ti_vals[i+1].value); // based on op, decide value of result. - if(op == TokenType::IS || op == TokenType::ISNOT) { + // `x is y` in Python is equivalent to `id(x) == id(y)` + // so we just compare pointers for equality and return the corresponding PyBool. + assert(i+1 < ti_vals.size()); bool finalResult = (ti_vals[i+1].value == res.value); // invert result if op is ISNOT. diff --git a/tuplex/test/core/IsKeywordTest.cc b/tuplex/test/core/IsKeywordTest.cc index 2ebf3ded6..acab0da97 100644 --- a/tuplex/test/core/IsKeywordTest.cc +++ b/tuplex/test/core/IsKeywordTest.cc @@ -1,3 +1,12 @@ +//--------------------------------------------------------------------------------------------------------------------// +// // +// Tuplex: Blazing Fast Python Data Science // +// // +// // +// (c) 2017 - 2021, Tuplex team // +// Created by Shreeyash Gotmare first on 10/1/2021 // +// License: Apache 2.0 // +//--------------------------------------------------------------------------------------------------------------------// #include "gtest/gtest.h" #include @@ -200,7 +209,7 @@ TEST_F(IsKeywordTest, MultipleIs) { Row row2(false); Row row3("hi"); - auto code = "lambda x: x is \"hi\" != False"; + auto code = "lambda x: x is \"hi\" is False"; auto node = UDF(code); @@ -210,3 +219,73 @@ TEST_F(IsKeywordTest, MultipleIs) { auto log = logStream.str(); EXPECT_NE(log.find("use of is comparison only supported with types"), std::string::npos); } + +TEST_F(IsKeywordTest, MultipleIs2) { + using namespace tuplex; + + Context c(microTestOptions()); + + Row row1(true); + Row row2(false); + Row row3(false); + + // equivalent to (x is "hi") and ("hi" is not True) => (x is "hi") and True => (x is "hi") + auto code = "lambda x: x is \"hi\" is not True"; + + auto node = UDF(code); + + auto m = c.parallelize({row1, row2, row3}) + .map(node).collectAsVector(); + + EXPECT_EQ(m.size(), 3); + assert(m[0].toPythonString() == "(False,)"); + assert(m[1].toPythonString() == "(False,)"); + assert(m[2].toPythonString() == "(False,)"); +} + +TEST_F(IsKeywordTest, MultipleIs3) { + using namespace tuplex; + + Context c(microTestOptions()); + + Row row1(Field(option(true))); + Row row2(Field(option(false))); + Row row3(Field(option::none)); + + // equivalent to (x is None) and (None is None) and (None is None) => (x is None). + auto code = "lambda x: x is None is None is None"; + + auto node = UDF(code); + + auto m = c.parallelize({row1, row2, row3}) + .map(node).collectAsVector(); + + EXPECT_EQ(m.size(), 3); + assert(m[0].toPythonString() == "(False,)"); + assert(m[1].toPythonString() == "(False,)"); + assert(m[2].toPythonString() == "(True,)"); +} + + +TEST_F(IsKeywordTest, MultipleIs4) { + using namespace tuplex; + + Context c(microTestOptions()); + + Row row1(Field(option(true))); + Row row2(Field(option(false))); + Row row3(Field(option::none)); + + // equivalent to (x is None) and (None is True) => (x is None) and False => False. + auto code = "lambda x: x is None is True"; + + auto node = UDF(code); + + auto m = c.parallelize({row1, row2, row3}) + .map(node).collectAsVector(); + + EXPECT_EQ(m.size(), 3); + assert(m[0].toPythonString() == "(False,)"); + assert(m[1].toPythonString() == "(False,)"); + assert(m[2].toPythonString() == "(False,)"); +} From fd75db3ee47739222353aa1d136b8ae81c9d5f80 Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Thu, 28 Oct 2021 10:47:41 -0400 Subject: [PATCH 12/14] nit --- tuplex/core/src/TraceVisitor.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/tuplex/core/src/TraceVisitor.cc b/tuplex/core/src/TraceVisitor.cc index e5056148d..bd2b79287 100644 --- a/tuplex/core/src/TraceVisitor.cc +++ b/tuplex/core/src/TraceVisitor.cc @@ -422,7 +422,6 @@ namespace tuplex { res.value = PyObject_RichCompare(res.value, ti_vals[i + 1].value, opid); } - // res.value = PyObject_RichCompare(res.value, ti_vals[i + 1].value, opid); auto res_info = "is: " + python::PyString_AsString(res.value); // NULL? ==> failure! assert(res.value); From 69ad71ed71b8cc2e7d6a8e2140d0a159f25d3ba4 Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Thu, 28 Oct 2021 12:33:19 -0400 Subject: [PATCH 13/14] fix tracer test bug --- tuplex/core/src/TraceVisitor.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tuplex/core/src/TraceVisitor.cc b/tuplex/core/src/TraceVisitor.cc index bd2b79287..630ceb7f1 100644 --- a/tuplex/core/src/TraceVisitor.cc +++ b/tuplex/core/src/TraceVisitor.cc @@ -399,14 +399,6 @@ namespace tuplex { // eval for(int i = 0; i < node->_ops.size(); ++i) { auto op = node->_ops[i]; - auto it = cmpLookup.find(op); - if(it == cmpLookup.end()) - throw std::runtime_error("Operator " + opToString(op) + " not yet supported in TraceVisitor/NCompare"); - int opid = it->second; - - // debug: - auto info = python::PyString_AsString(res.value) + " " + opToString(op) + " " + - python::PyString_AsString(ti_vals[i+1].value); // based on op, decide value of result. if(op == TokenType::IS || op == TokenType::ISNOT) { @@ -419,9 +411,18 @@ namespace tuplex { finalResult = (op == TokenType::IS) ? finalResult : !finalResult; res.value = finalResult ? Py_True : Py_False; } else { + auto it = cmpLookup.find(op); + if(it == cmpLookup.end()) + throw std::runtime_error("Operator " + opToString(op) + " not yet supported in TraceVisitor/NCompare"); + int opid = it->second; + res.value = PyObject_RichCompare(res.value, ti_vals[i + 1].value, opid); } + // debug: + auto info = python::PyString_AsString(res.value) + " " + opToString(op) + " " + + python::PyString_AsString(ti_vals[i+1].value); + auto res_info = "is: " + python::PyString_AsString(res.value); // NULL? ==> failure! assert(res.value); From dad49eaed89f6ab7f324ad507467b5bef84ac380 Mon Sep 17 00:00:00 2001 From: Yash Gotmare Date: Thu, 28 Oct 2021 12:53:16 -0400 Subject: [PATCH 14/14] remove debug --- tuplex/core/src/TraceVisitor.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tuplex/core/src/TraceVisitor.cc b/tuplex/core/src/TraceVisitor.cc index 630ceb7f1..ecaf37c09 100644 --- a/tuplex/core/src/TraceVisitor.cc +++ b/tuplex/core/src/TraceVisitor.cc @@ -419,11 +419,6 @@ namespace tuplex { res.value = PyObject_RichCompare(res.value, ti_vals[i + 1].value, opid); } - // debug: - auto info = python::PyString_AsString(res.value) + " " + opToString(op) + " " + - python::PyString_AsString(ti_vals[i+1].value); - - auto res_info = "is: " + python::PyString_AsString(res.value); // NULL? ==> failure! assert(res.value); }