Fix unsoundness in `alter space` and remove redundant macro usage

server/src/engine/ql/ast/mod.rs

@@ -302,11 +302,11 @@ impl<'a> QueryData<'a> for InplaceData {
     }
     #[inline(always)]
     unsafe fn read_lit(&mut self, tok: &'a Token) -> LitIR<'a> {
-        extract!(tok, Token::Lit(l) => l.as_ir())
+        tok.uck_read_lit().as_ir()
     }
     #[inline(always)]
     unsafe fn read_data_type(&mut self, tok: &'a Token) -> Datacell {
-        Datacell::from(extract!(tok, Token::Lit(ref l) => l.to_owned()))
+        Datacell::from(<Self as QueryData>::read_lit(self, tok))
     }
     #[inline(always)]
     fn nonzero(&self) -> bool {
@@ -384,10 +384,7 @@ impl<'a> Entity<'a> {
     /// Caller guarantees that the token stream matches the exact stream of tokens
     /// expected for a full entity
     pub(super) unsafe fn full_entity_from_slice(sl: &'a [Token]) -> Self {
-        Entity::Full(
-            extract!(&sl[0], Token::Ident(sl) => sl.clone()),
-            extract!(&sl[2], Token::Ident(sl) => sl.clone()),
-        )
+        Entity::Full(sl[0].uck_read_ident(), sl[1].uck_read_ident())
     }
     #[inline(always)]
     /// Parse a single entity from the given slice
@@ -397,7 +394,7 @@ impl<'a> Entity<'a> {
     /// Caller guarantees that the token stream matches the exact stream of tokens
     /// expected for a single entity
     pub(super) unsafe fn single_entity_from_slice(sl: &'a [Token]) -> Self {
-        Entity::Single(extract!(&sl[0], Token::Ident(sl) => sl.clone()))
+        Entity::Single(sl[0].uck_read_ident())
     }
     #[inline(always)]
     /// Returns true if the given token stream matches the signature of single entity syntax

server/src/engine/ql/ddl/alt.rs

@@ -60,6 +60,7 @@ impl<'a> AlterSpace<'a> {
             return compiler::cold_rerr(LangError::UnexpectedEOS);
         }
         let space_name = state.fw_read();
+        state.poison_if_not(space_name.is_ident());
         state.poison_if_not(state.cursor_eq(Token![with]));
         state.cursor_ahead(); // ignore errors
         state.poison_if_not(state.cursor_eq(Token![open {}]));
@@ -69,7 +70,10 @@ impl<'a> AlterSpace<'a> {
             return Err(LangError::BadSyntax);
         }
 
-        let space_name = unsafe { extract!(space_name, Token::Ident(ref space) => space.clone()) };
+        let space_name = unsafe {
+            // UNSAFE(@ohsayan): We just verified that `space_name` is an ident
+            space_name.uck_read_ident()
+        };
         let mut d = DictGeneric::new();
         syn::rfold_dict(DictFoldState::CB_OR_IDENT, state, &mut d);
         if state.okay() {
@@ -113,7 +117,7 @@ impl<'a> AlterModel<'a> {
             return compiler::cold_rerr(LangError::BadSyntax);
             // FIXME(@ohsayan): bad because no specificity
         }
-        let model_name = unsafe { extract!(state.fw_read(), Token::Ident(ref l) => l.clone()) };
+        let model_name = unsafe { state.fw_read().uck_read_ident() };
         let kind = match state.fw_read() {
             Token![add] => AlterKind::alter_add(state),
             Token![remove] => AlterKind::alter_remove(state),

server/src/engine/ql/ddl/crt.rs

@@ -32,7 +32,7 @@ use {
             error::{LangError, LangResult},
             ql::{
                 ast::{QueryData, State},
-                lex::{Ident, Token},
+                lex::Ident,
             },
         },
         util::compiler,
@@ -69,7 +69,10 @@ impl<'a> CreateSpace<'a> {
         }
         if state.okay() {
             Ok(CreateSpace {
-                space_name: unsafe { extract!(space_name, Token::Ident(ref id) => id.clone()) },
+                space_name: unsafe {
+                    // UNSAFE(@ohsayan): we checked if `space_name` with `is_ident` above
+                    space_name.uck_read_ident()
+                },
                 props: d,
             })
         } else {
@@ -109,9 +112,9 @@ impl<'a> CreateModel<'a> {
         if compiler::unlikely(state.remaining() < 10) {
             return compiler::cold_rerr(LangError::UnexpectedEOS);
         }
-        // field name; ignore errors
-        let field_name = state.fw_read();
-        state.poison_if_not(field_name.is_ident());
+        // model name; ignore errors
+        let model_name = state.fw_read();
+        state.poison_if_not(model_name.is_ident());
         state.poison_if_not(state.cursor_eq(Token![() open]));
         state.cursor_ahead();
         // fields
@@ -136,7 +139,10 @@ impl<'a> CreateModel<'a> {
         // we're done
         if state.okay() {
             Ok(Self {
-                model_name: unsafe { extract!(field_name, Token::Ident(id) => *id) },
+                model_name: unsafe {
+                    // UNSAFE(@ohsayan): we verified if `model_name` returns `is_ident`
+                    model_name.uck_read_ident()
+                },
                 fields,
                 props,
             })

server/src/engine/ql/ddl/drop.rs

@@ -56,7 +56,7 @@ impl<'a> DropSpace<'a> {
                 return Ok(DropSpace::new(
                     unsafe {
                         // UNSAFE(@ohsayan): Safe because the match predicate ensures that tok[1] is indeed an ident
-                        extract!(ident, Token::Ident(ref space) => *space)
+                        ident.uck_read_ident()
                     },
                     force,
                 ));

server/src/engine/ql/ddl/ins.rs

@@ -55,7 +55,7 @@ pub fn parse_inspect<'a, Qd: QueryData<'a>>(
         Token![space] if state.cursor_has_ident_rounded() => {
             Ok(Statement::InspectSpace(unsafe {
                 // UNSAFE(@ohsayan): Safe because of the match predicate
-                extract!(state.fw_read(), Token::Ident(ref space) => *space)
+                state.fw_read().uck_read_ident()
             }))
         }
         Token::Ident(id) if id.eq_ignore_ascii_case("spaces") && state.exhausted() => {

server/src/engine/ql/ddl/syn.rs

@@ -418,7 +418,10 @@ impl<'a> ExpandedField<'a> {
         }
         if state.okay() {
             Ok(Self {
-                field_name: unsafe { extract!(field_name, Token::Ident(id) => *id) },
+                field_name: unsafe {
+                    // UNSAFE(@ohsayan): We just verified if `field_name` returns `is_ident`
+                    field_name.uck_read_ident()
+                },
                 props,
                 layers,
             })

server/src/engine/ql/dml/ins.rs

@@ -25,7 +25,6 @@
 */
 
 use {
-    super::read_ident,
     crate::{
         engine::{
             core::Datacell,
@@ -200,7 +199,7 @@ pub(super) fn parse_list<'a, Qd: QueryData<'a>>(
 /// ## Safety
 /// - Cursor must match arity(0) function signature
 unsafe fn handle_func_sub<'a, Qd: QueryData<'a>>(state: &mut State<'a, Qd>) -> Option<Datacell> {
-    let func = read_ident(state.fw_read());
+    let func = state.fw_read().uck_read_ident();
     state.cursor_ahead_by(2); // skip tt:paren
     ldfunc(func).map(move |f| f())
 }

server/src/engine/ql/dml/mod.rs

@@ -37,17 +37,12 @@ pub mod upd;
 use {
     super::{
         ast::{QueryData, State},
-        lex::{Ident, Token},
+        lex::Ident,
     },
     crate::{engine::data::lit::LitIR, util::compiler},
     std::collections::HashMap,
 };
 
-#[inline(always)]
-unsafe fn read_ident<'a>(tok: &'a Token<'a>) -> Ident<'a> {
-    extract!(tok, Token::Ident(ref tok) => *tok)
-}
-
 #[inline(always)]
 fn u(b: bool) -> u8 {
     b as _
@@ -108,9 +103,11 @@ impl<'a> RelationalExpr<'a> {
         state.poison_if_not(state.can_read_lit_rounded());
         if compiler::likely(state.okay()) {
             unsafe {
+                // UNSAFE(@ohsayan): we verified this above
                 let lit = state.read_cursor_lit_unchecked();
                 state.cursor_ahead();
-                Some(Self::new(read_ident(ident), lit, operator))
+                // UNSAFE(@ohsayan): we checked if `ident` returns `is_ident` and updated state
+                Some(Self::new(ident.uck_read_ident(), lit, operator))
             }
         } else {
             None

server/src/engine/ql/dml/upd.rs

@@ -27,7 +27,7 @@
 #[cfg(test)]
 use super::WhereClauseCollection;
 use {
-    super::{read_ident, u, WhereClause},
+    super::{u, WhereClause},
     crate::{
         engine::{
             data::lit::LitIR,
@@ -123,7 +123,8 @@ impl<'a> AssignmentExpression<'a> {
                 let rhs = state.read_cursor_lit_unchecked();
                 state.cursor_ahead();
                 expressions.push(AssignmentExpression::new(
-                    read_ident(lhs),
+                    // UNSAFE(@ohsayan): we verified if `lhs` returns `is_ident`
+                    lhs.uck_read_ident(),
                     rhs,
                     OPERATOR[operator_code as usize],
                 ))

server/src/engine/ql/lex/raw.rs

@@ -124,6 +124,15 @@ pub enum Token<'a> {
     Lit(Lit<'a>), // literal
 }
 
+impl<'a> Token<'a> {
+    pub unsafe fn uck_read_ident(&self) -> Ident<'a> {
+        extract!(self, Self::Ident(id) => *id)
+    }
+    pub unsafe fn uck_read_lit(&self) -> &Lit<'a> {
+        extract!(self, Self::Lit(l) => l)
+    }
+}
+
 impl<'a> ToString for Token<'a> {
     fn to_string(&self) -> String {
         match self {