Skip to content

Fix logical comparison #2598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 26 commits into from
Mar 18, 2024
Merged
Changes from 8 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3e87bc9
Fix logical comparison
kmr-srbh Mar 11, 2024
e25820b
Merge branch 'main' into fix-logical-compare
kmr-srbh Mar 11, 2024
b79daf5
Handle logical comparisons on non-logical expressions
kmr-srbh Mar 17, 2024
94530a2
Handle logical comparison on strings
kmr-srbh Mar 17, 2024
bc2de09
Fix type comparison
kmr-srbh Mar 17, 2024
bcc1c42
Fix nullptr error
kmr-srbh Mar 17, 2024
5a3213b
Fix string comparison
kmr-srbh Mar 17, 2024
2eb2edd
Merge branch 'main' into fix-logical-compare
kmr-srbh Mar 17, 2024
2376719
Evaluate logical operations at compile time
kmr-srbh Mar 17, 2024
60a4a1b
Merge branch 'main' into fix-logical-compare
kmr-srbh Mar 17, 2024
46ab7be
Merge branch 'main' into fix-logical-compare
kmr-srbh Mar 18, 2024
64b1fd2
Tests: Add error tests
kmr-srbh Mar 18, 2024
1013838
Tests: Add tests
kmr-srbh Mar 18, 2024
430eca4
Tests: Update test references
kmr-srbh Mar 18, 2024
2473f76
Move `tmp`
kmr-srbh Mar 18, 2024
6441912
Tests: Update test references
kmr-srbh Mar 18, 2024
5aebcb6
Add check for empty function call
kmr-srbh Mar 18, 2024
cc5aaa8
Tests: Update tests and test references
kmr-srbh Mar 18, 2024
94041a2
Tests: Fix errors
kmr-srbh Mar 18, 2024
e644814
Delete tests/reference/asr-test_logical_assignment-6b36ea2.json
kmr-srbh Mar 18, 2024
4973f05
Delete tests/reference/asr-test_logical_assignment-6b36ea2.stderr
kmr-srbh Mar 18, 2024
a827e1c
Delete tests/reference/asr-test_logical_compare-d8a2a03.json
kmr-srbh Mar 18, 2024
332a987
Delete tests/reference/asr-test_logical_compare_02-878af11.stderr
kmr-srbh Mar 18, 2024
cd26f8f
Delete tests/reference/asr-test_logical_compare_02-878af11.json
kmr-srbh Mar 18, 2024
74b8d51
Delete tests/reference/asr-test_logical_compare-d8a2a03.stderr
kmr-srbh Mar 18, 2024
a756338
Tests: Add error test to tests.toml
kmr-srbh Mar 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 36 additions & 17 deletions src/lpython/semantics/python_ast_to_asr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3326,29 +3326,48 @@ class CommonVisitor : public AST::BaseVisitor<Struct> {
x.base.base.loc);
}
}
LCOMPILERS_ASSERT(
ASRUtils::check_equal_type(ASRUtils::expr_type(lhs), ASRUtils::expr_type(rhs)));
ASR::ttype_t *left_operand_type = ASRUtils::expr_type(lhs);
ASR::ttype_t *right_operand_type = ASRUtils::expr_type(rhs);

ASR::expr_t *value = nullptr;
ASR::ttype_t *dest_type = ASRUtils::expr_type(lhs);
ASR::ttype_t *dest_type = left_operand_type;

if (ASR::is_a<ASR::Character_t>(*left_operand_type)) {
throw SemanticError("Logical operation not supported on object of type 'str'", lhs->base.loc);
}
if (ASR::is_a<ASR::Character_t>(*right_operand_type)) {
throw SemanticError("Logical operation not supported on object of type 'str'", rhs->base.loc);
}
if (ASRUtils::expr_value(lhs) != nullptr && ASRUtils::expr_value(rhs) != nullptr) {
if (ASR::is_a<ASR::Logical_t>(*left_operand_type)
&& ASR::is_a<ASR::Logical_t>(*right_operand_type)) {

LCOMPILERS_ASSERT(ASR::is_a<ASR::Logical_t>(*dest_type));
bool left_value = ASR::down_cast<ASR::LogicalConstant_t>(
ASRUtils::expr_value(lhs))->m_value;
bool right_value = ASR::down_cast<ASR::LogicalConstant_t>(
ASRUtils::expr_value(rhs))->m_value;
bool result;
switch (op) {
case (ASR::logicalbinopType::And): { result = left_value && right_value; break; }
case (ASR::logicalbinopType::Or): { result = left_value || right_value; break; }
default : {
throw SemanticError("Boolean operator type not supported",
x.base.base.loc);
bool left_value = ASR::down_cast<ASR::LogicalConstant_t>(
ASRUtils::expr_value(lhs))->m_value;
bool right_value = ASR::down_cast<ASR::LogicalConstant_t>(
ASRUtils::expr_value(rhs))->m_value;
bool result;
switch (op) {
case (ASR::logicalbinopType::And): { result = left_value && right_value; break; }
case (ASR::logicalbinopType::Or): { result = left_value || right_value; break; }
default : {
throw SemanticError("Boolean operator type not supported",
x.base.base.loc);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change was made in this section. Only indentation was added.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems an assert LCOMPILERS_ASSERT(ASR::is_a<ASR::Logical_t>(*dest_type)); is removed. I suggest not doing indentation changes in this PR. Instead you can submit a separate PR for indentation changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shaikh-Ubaid Removal of that assert statement was intentional. In this case, indentation was required to add readability to the if block.

}
value = ASR::down_cast<ASR::expr_t>(ASR::make_LogicalConstant_t(
al, x.base.base.loc, result, dest_type));
} else if(ASR::is_a<ASR::Logical_t>(*left_operand_type)
&& !ASR::is_a<ASR::Logical_t>(*right_operand_type)) {
throw SemanticError("Type mismatch: '" + ASRUtils::type_to_str_python(left_operand_type)
+ "' and '" + ASRUtils::type_to_str_python(right_operand_type)
+ "'. Operand should be of type 'bool'", rhs->base.loc);
} else if(!ASR::is_a<ASR::Logical_t>(*left_operand_type)
&& ASR::is_a<ASR::Logical_t>(*right_operand_type)) {
throw SemanticError("Type mismatch: '" + ASRUtils::type_to_str_python(left_operand_type)
+ "' and '" + ASRUtils::type_to_str_python(right_operand_type)
+ "'. Operand should be of type 'bool'", lhs->base.loc);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value = ASR::down_cast<ASR::expr_t>(ASR::make_LogicalConstant_t(
al, x.base.base.loc, result, dest_type));
}
tmp = ASR::make_LogicalBinOp_t(al, x.base.base.loc, lhs, op, rhs, dest_type, value);
}
Expand Down