Skip to content

Commit

Permalink
Interpret PrefixExpr of literal ints and floats into ASG
Browse files Browse the repository at this point in the history
This correctly parses and analyzes to ASG some expressions representing negative numeric literals.
At the same time it does not incorrectly parse binary expressions such as `a-1.23`
The following (should!) enter the ASG correctly:
`1.23;`
`-1.23;`
`- 1.23;`
`1;`
`-1;`
`- 1`;

Some tests for all of the items above (and the binary expression) are included in this commit.

There are a few ugly things. But maybe they can be fixed later.

This PR also should make it easier to do anoether PR to handle variables with unary minus.

The access (API) to the ASG, tries to be backward compatible. Probably could used a
simple redesign.

* A new accessor `sign()` is added to `IntLiteral`. For positive values `sign()` returns `true`,
  for negative `false`.

* `FloatLiteral` is still a `String` and the possible `-` sign is formatted into the string.
   We talked about storing an f64 here instead. Probably would be a good option. But the present
   API is backward compatible.
  • Loading branch information
jlapeyre committed Feb 1, 2024
1 parent 561c79e commit 73f300a
Show file tree
Hide file tree
Showing 9 changed files with 211 additions and 11 deletions.
9 changes: 7 additions & 2 deletions crates/oq3_semantics/src/asg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,21 +782,26 @@ impl BoolLiteral {
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct IntLiteral {
value: u128,
sign: bool,
}

impl IntLiteral {
pub fn new<T>(value: T) -> IntLiteral
pub fn new<T>(value: T, sign: bool) -> IntLiteral
where
T: Into<u128>,
{
let x: u128 = value.into();
IntLiteral { value: x }
IntLiteral { value: x, sign }
}

pub fn value(&self) -> &u128 {
&self.value
}

pub fn sign(&self) -> &bool {
&self.sign
}

pub fn to_expr(self) -> Expr {
Expr::Literal(Literal::Int(self))
}
Expand Down
27 changes: 26 additions & 1 deletion crates/oq3_semantics/src/syntax_to_semantics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,31 @@ fn from_paren_expr(paren_expr: synast::ParenExpr, context: &mut Context) -> Opti

fn from_expr(expr: synast::Expr, context: &mut Context) -> Option<asg::TExpr> {
match expr {
// FIXME: Ugh. could clean up logic here
synast::Expr::PrefixExpr(prefix_expr) => {
match prefix_expr.op_kind()? {
synast::UnaryOp::Neg => {
if let synast::Expr::Literal(ref literal) = prefix_expr.expr()? {
match literal.kind() {
synast::LiteralKind::FloatNumber(float_num) => {
let num = float_num.value().unwrap();
let float = format!("-{num}");
return Some(asg::FloatLiteral::new(float).to_texpr());
}
synast::LiteralKind::IntNumber(int_num) => {
let num = int_num.value_u128().unwrap(); // fn value_u128 is kind of a hack
return Some(asg::IntLiteral::new(num, false).to_texpr());
// `false` means negative
}
_ => todo!(),
}
}
None
}
_ => None,
}
}

synast::Expr::ParenExpr(paren_expr) => from_paren_expr(paren_expr, context),

synast::Expr::BinExpr(bin_expr) => {
Expand Down Expand Up @@ -469,7 +494,7 @@ fn from_literal(literal: &synast::Literal) -> Option<asg::TExpr> {

synast::LiteralKind::IntNumber(int_num) => {
let num = int_num.value_u128().unwrap(); // fn value_u128 is kind of a hack
asg::IntLiteral::new(num).to_texpr()
asg::IntLiteral::new(num, true).to_texpr() // `true` means positive literal.
}

synast::LiteralKind::FloatNumber(float_num) => {
Expand Down
6 changes: 3 additions & 3 deletions crates/oq3_semantics/tests/ast_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn test_texpr_int_literal() {
use asg::IntLiteral;
use types::{IsConst, Type};

let literal = IntLiteral::new(1_u32);
let literal = IntLiteral::new(1_u32, true);
let texpr = literal.clone().to_texpr();
assert_eq!(texpr.expression(), &literal.to_expr());
assert_eq!(texpr.get_type(), &Type::UInt(Some(128), IsConst::True));
Expand All @@ -47,7 +47,7 @@ fn test_int_literal() {
use asg::IntLiteral;

let int_value = 42;
let literal = IntLiteral::new(int_value as u32);
let literal = IntLiteral::new(int_value as u32, true);
assert_eq!(*literal.value(), int_value as u128);
}

Expand Down Expand Up @@ -75,7 +75,7 @@ fn test_cast() {
use types::{IsConst, Type};

let typ = Type::Int(Some(32), IsConst::True);
let literal = IntLiteral::new(1_u64).to_texpr();
let literal = IntLiteral::new(1_u64, true).to_texpr();
let cast = Cast::new(literal.clone(), typ.clone());
assert_eq!(cast.get_type(), &typ);
assert_eq!(cast.operand(), &literal);
Expand Down
111 changes: 111 additions & 0 deletions crates/oq3_semantics/tests/from_string_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,3 +345,114 @@ out[0] = measure $0;
assert!(errors.is_empty());
assert_eq!(program.len(), 2);
}

fn expr_from_expr_stmt(stmt: &asg::Stmt) -> asg::Expr {
match stmt {
asg::Stmt::ExprStmt(texpr) => texpr.expression().clone(),
_ => unreachable!(),
}
}

fn literal_value(stmt: &asg::Stmt) -> Option<String> {
match expr_from_expr_stmt(stmt) {
asg::Expr::Literal(lit) => match lit {
asg::Literal::Float(float) => Some(float.value().to_string()),
asg::Literal::Int(int) => {
if *int.sign() {
Some(format!("{}", int.value()))
} else {
Some(format!("-{}", int.value()))
}
}
_ => None,
},
_ => None,
}
}

#[test]
fn test_from_string_pos_lit_float() {
let code = r##"
1.23;
"##;
let (program, errors, _symbol_table) = parse_string(code);
assert!(errors.is_empty());
assert_eq!(program.len(), 1);
let expr = literal_value(&program[0]).unwrap();
assert_eq!(expr, "1.23");
}

#[test]
fn test_from_string_neg_lit_float() {
let code = r##"
-1.23;
"##;
let (program, errors, _symbol_table) = parse_string(code);
assert!(errors.is_empty());
assert_eq!(program.len(), 1);
let expr = literal_value(&program[0]).unwrap();
assert_eq!(expr, "-1.23");
}

#[test]
fn test_from_string_pos_lit_int() {
let code = r##"
123;
"##;
let (program, errors, _symbol_table) = parse_string(code);
assert!(errors.is_empty());
assert_eq!(program.len(), 1);
let expr = literal_value(&program[0]).unwrap();
assert_eq!(expr, "123");
}

#[test]
fn test_from_string_neg_lit_int() {
let code = r##"
-123;
"##;
let (program, errors, _symbol_table) = parse_string(code);
assert!(errors.is_empty());
assert_eq!(program.len(), 1);
let expr = literal_value(&program[0]).unwrap();
assert_eq!(expr, "-123");
}

#[test]
fn test_from_string_neg_spc_lit_int() {
let code = r##"
- 123;
"##;
let (program, errors, _symbol_table) = parse_string(code);
assert!(errors.is_empty());
assert_eq!(program.len(), 1);
let expr = literal_value(&program[0]).unwrap();
assert_eq!(expr, "-123");
}

#[test]
fn test_from_string_neg_spc_lit_float() {
let code = r##"
- 1.23;
"##;
let (program, errors, _symbol_table) = parse_string(code);
assert!(errors.is_empty());
assert_eq!(program.len(), 1);
let expr = literal_value(&program[0]).unwrap();
assert_eq!(expr, "-1.23");
}

// PR #91
#[test]
fn test_from_string_bin_expr_no_spc() {
let code = r##"
a-1.23;
"##;
let (program, errors, _symbol_table) = parse_string(code);
assert_eq!(errors.len(), 1);
assert_eq!(program.len(), 1);
assert!(matches!(
expr_from_expr_stmt(&program[0]),
asg::Expr::BinaryExpr(_)
));
}
4 changes: 4 additions & 0 deletions crates/oq3_syntax/openqasm3.ungram
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ Expr =
| MeasureExpression
| ModifiedGateCallExpr
| ParenExpr
| PrefixExpr
| RangeExpr
| ReturnExpr

Expand Down Expand Up @@ -243,6 +244,9 @@ BlockExpr =
statements:Stmt*
'}'

PrefixExpr =
op:('~' | '!' | '-') Expr

// Adding a binary op here requires changes in several places. See notes at the top of this file.
BinExpr =
lhs:Expr
Expand Down
18 changes: 17 additions & 1 deletion crates/oq3_syntax/src/ast/expr_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use crate::{
ast::{
self,
operators::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering},
operators::{ArithOp, BinaryOp, CmpOp, LogicOp, Ordering, UnaryOp},
support, AstChildren, AstNode,
},
AstToken,
Expand Down Expand Up @@ -147,6 +147,22 @@ impl ast::IfStmt {
// assert_eq!(else_.syntax().text(), r#"{ "else" }"#);
// }

impl ast::PrefixExpr {
pub fn op_kind(&self) -> Option<UnaryOp> {
let res = match self.op_token()?.kind() {
T![~] => UnaryOp::LogicNot,
T![!] => UnaryOp::Not,
T![-] => UnaryOp::Neg,
_ => return None,
};
Some(res)
}

pub fn op_token(&self) -> Option<SyntaxToken> {
self.syntax().first_child_or_token()?.into_token()
}
}

impl ast::BinExpr {
pub fn op_details(&self) -> Option<(SyntaxToken, BinaryOp)> {
self.syntax().children_with_tokens().filter_map(|it| it.into_token()).find_map(|c| {
Expand Down
38 changes: 38 additions & 0 deletions crates/oq3_syntax/src/ast/generated/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,15 @@ impl ParenExpr {
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct PrefixExpr {
pub(crate) syntax: SyntaxNode,
}
impl PrefixExpr {
pub fn expr(&self) -> Option<Expr> {
support::child(&self.syntax)
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct RangeExpr {
pub(crate) syntax: SyntaxNode,
}
Expand Down Expand Up @@ -1058,6 +1067,7 @@ pub enum Expr {
MeasureExpression(MeasureExpression),
ModifiedGateCallExpr(ModifiedGateCallExpr),
ParenExpr(ParenExpr),
PrefixExpr(PrefixExpr),
RangeExpr(RangeExpr),
ReturnExpr(ReturnExpr),
}
Expand Down Expand Up @@ -1834,6 +1844,21 @@ impl AstNode for ParenExpr {
&self.syntax
}
}
impl AstNode for PrefixExpr {
fn can_cast(kind: SyntaxKind) -> bool {
kind == PREFIX_EXPR
}
fn cast(syntax: SyntaxNode) -> Option<Self> {
if Self::can_cast(syntax.kind()) {
Some(Self { syntax })
} else {
None
}
}
fn syntax(&self) -> &SyntaxNode {
&self.syntax
}
}
impl AstNode for RangeExpr {
fn can_cast(kind: SyntaxKind) -> bool {
kind == RANGE_EXPR
Expand Down Expand Up @@ -2425,6 +2450,11 @@ impl From<ParenExpr> for Expr {
Expr::ParenExpr(node)
}
}
impl From<PrefixExpr> for Expr {
fn from(node: PrefixExpr) -> Expr {
Expr::PrefixExpr(node)
}
}
impl From<RangeExpr> for Expr {
fn from(node: RangeExpr) -> Expr {
Expr::RangeExpr(node)
Expand Down Expand Up @@ -2455,6 +2485,7 @@ impl AstNode for Expr {
| MEASURE_EXPRESSION
| MODIFIED_GATE_CALL_EXPR
| PAREN_EXPR
| PREFIX_EXPR
| RANGE_EXPR
| RETURN_EXPR
)
Expand All @@ -2477,6 +2508,7 @@ impl AstNode for Expr {
MEASURE_EXPRESSION => Expr::MeasureExpression(MeasureExpression { syntax }),
MODIFIED_GATE_CALL_EXPR => Expr::ModifiedGateCallExpr(ModifiedGateCallExpr { syntax }),
PAREN_EXPR => Expr::ParenExpr(ParenExpr { syntax }),
PREFIX_EXPR => Expr::PrefixExpr(PrefixExpr { syntax }),
RANGE_EXPR => Expr::RangeExpr(RangeExpr { syntax }),
RETURN_EXPR => Expr::ReturnExpr(ReturnExpr { syntax }),
_ => return None,
Expand All @@ -2501,6 +2533,7 @@ impl AstNode for Expr {
Expr::MeasureExpression(it) => &it.syntax,
Expr::ModifiedGateCallExpr(it) => &it.syntax,
Expr::ParenExpr(it) => &it.syntax,
Expr::PrefixExpr(it) => &it.syntax,
Expr::RangeExpr(it) => &it.syntax,
Expr::ReturnExpr(it) => &it.syntax,
}
Expand Down Expand Up @@ -2978,6 +3011,11 @@ impl std::fmt::Display for ParenExpr {
std::fmt::Display::fmt(self.syntax(), f)
}
}
impl std::fmt::Display for PrefixExpr {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(self.syntax(), f)
}
}
impl std::fmt::Display for RangeExpr {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(self.syntax(), f)
Expand Down
4 changes: 2 additions & 2 deletions crates/oq3_syntax/src/ast/operators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ pub enum RangeOp {

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub enum UnaryOp {
/// `*`
Deref,
/// `~`
LogicNot,
/// `!`
Not,
/// `-`
Expand Down
5 changes: 3 additions & 2 deletions crates/oq3_syntax/src/ast/prec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl Expr {
}
ArrayLiteral(_) => (0, 0), // These need to be checked
MeasureExpression(_) => (0, 0),
BoxExpr(_) => (0, 27),
BoxExpr(_) | PrefixExpr(_) => (0, 27),
GateCallExpr(_)
| ModifiedGateCallExpr(_)
| CallExpr(_)
Expand Down Expand Up @@ -191,6 +191,7 @@ impl Expr {
// For non-paren-like operators: get the operator itself
let token = match this {
RangeExpr(_) => None,
PrefixExpr(e) => e.op_token(),
BinExpr(e) => e.op_token(),
BoxExpr(e) => e.box_token(),
CallExpr(e) => e.arg_list().and_then(|args| args.l_paren_token()),
Expand Down Expand Up @@ -241,7 +242,7 @@ impl Expr {
| ParenExpr(_) => false,

// For BinExpr and RangeExpr this is technically wrong -- the child can be on the left...
BinExpr(_) | RangeExpr(_) | BoxExpr(_) | ReturnExpr(_) => self
PrefixExpr(_) | BinExpr(_) | RangeExpr(_) | BoxExpr(_) | ReturnExpr(_) => self
.syntax()
.parent()
.and_then(Expr::cast)
Expand Down

0 comments on commit 73f300a

Please sign in to comment.