Skip to content

Commit

Permalink
Earlier const-checking during IR rather than GD
Browse files Browse the repository at this point in the history
  • Loading branch information
Mercerenies committed Oct 27, 2022
1 parent 9223a06 commit ca4ae3c
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 19 deletions.
57 changes: 57 additions & 0 deletions src/compile/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ use crate::gdscript::expr::{Expr, ExprF};
use crate::gdscript::op;
use crate::pipeline::source::{Sourced, SourceOffset};
use crate::ir::expr::{Expr as IRExpr, ExprF as IRExprF};
use crate::ir::decl::{ClassInnerDecl, ClassInnerDeclF, Decl, DeclF};
use crate::ir::literal::Literal;
use crate::ir::scope::decl::on_each_lambda_class;

use phf::phf_set;

Expand All @@ -28,6 +30,61 @@ pub const CONSTANT_GDSCRIPT_FUNCTIONS: phf::Set<&'static str> = phf_set! {
"Quat", "Basis", "Transform", "Color",
};

pub fn validate_all_constant_scopes(decls: &[Decl], table: &SymbolTable) -> Result<(), GDError> {

// Check all top-level constant and enum declarations, and delegate for top-level classes
for decl in decls {
match &decl.value {
DeclF::ConstDecl(const_decl) => {
validate_const_expr(&const_decl.name, &const_decl.value, table)?;
}
DeclF::EnumDecl(enum_decl) => {
for (_, rhs) in &enum_decl.clauses {
if let Some(rhs) = rhs {
validate_const_expr(&enum_decl.name, &rhs, table)?;
}
}
}
DeclF::ClassDecl(class_decl) => {
validate_constant_names_in_class(&class_decl.decls, table)?;
}
_ => {}
}
}

// Check all lambda classes
on_each_lambda_class(decls, |cls| {
validate_constant_names_in_class(&cls.decls, table)
})?;

Ok(())
}

fn validate_constant_names_in_class(inner_decls: &[ClassInnerDecl], table: &SymbolTable) -> Result<(), GDError> {
for decl in inner_decls {
match &decl.value {
ClassInnerDeclF::ClassConstDecl(const_decl) => {
validate_const_expr(&const_decl.name, &const_decl.value, table)?;
}
ClassInnerDeclF::ClassVarDecl(var_decl) => {
if let Some(export) = &var_decl.export {
for arg in &export.args {
// As a special exception, we allow any literal symbols
// appearing here, since they can reference things like
// `int` freely. (TODO Just generally make exports fit
// better with the rest of GDLisp)
if !(matches!(&arg.value, IRExprF::LocalVar(_))) {
validate_const_expr(&var_decl.name, arg, table)?;
}
}
}
}
_ => {}
}
}
Ok(())
}

pub fn validate_const_expr(name: &str, expr: &IRExpr, table: &SymbolTable) -> Result<(), GDError> {
match &expr.value {
IRExprF::LocalVar(name) => {
Expand Down
15 changes: 10 additions & 5 deletions src/compile/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use super::Compiler;
use super::factory;
use super::special_form;
use super::names;
use super::constant::validate_all_constant_scopes;
use super::special_form::lambda;
use super::special_form::flet;
use super::special_form::lambda_class;
Expand All @@ -20,7 +21,6 @@ use super::stateful::{StExpr, NeedsResult, SideEffects};
use super::body::builder::{StmtBuilder, CodeBuilder, HasDecls};
use super::body::class_scope::ClassScope;
use super::stmt_wrapper::{self, StmtWrapper};
use super::constant::MaybeConstant;
use crate::pipeline::Pipeline;
use crate::pipeline::error::PError;
use crate::pipeline::source::SourceOffset;
Expand Down Expand Up @@ -242,13 +242,22 @@ impl<'a, 'b, 'c, 'd, 'e> CompilerFrame<'a, 'b, 'c, 'd, 'e, CodeBuilder> {
}

pub fn compile_decls(&mut self, decls: &[IRDecl]) -> Result<(), GDError> {

// Bind all declarations into the symbol table.
for decl in decls {
Compiler::bind_decl(&self.compiler.magic_table, self.pipeline, self.table, decl)?;
}

// Validate the const-ness of any constants, enum values, or
// `export` clause bodies.
validate_all_constant_scopes(decls, self.table)?;

// Now compile.
for decl in decls {
self.table.clear_synthetic_locals();
self.compile_decl(decl)?;
}

Ok(())
}

Expand Down Expand Up @@ -281,7 +290,6 @@ impl<'a, 'b, 'c, 'd, 'e> CompilerFrame<'a, 'b, 'c, 'd, 'e, CodeBuilder> {
IRDeclF::ConstDecl(ir::decl::ConstDecl { visibility: _, name, value }) => {
let gd_name = names::lisp_to_gd(name);
let value = self.compile_simple_expr(name, value, NeedsResult::Yes)?;
value.validate_const_expr(name, self.table)?;
self.builder.add_decl(Decl::new(DeclF::ConstDecl(gd_name, value), decl.pos));
Ok(())
}
Expand Down Expand Up @@ -315,9 +323,6 @@ impl<'a, 'b, 'c, 'd, 'e> CompilerFrame<'a, 'b, 'c, 'd, 'e, CodeBuilder> {
let gd_clauses = clauses.iter().map(|(const_name, const_value)| {
let gd_const_name = names::lisp_to_gd(const_name);
let gd_const_value = const_value.as_ref().map(|x| self.compile_simple_expr(const_name, x, NeedsResult::Yes)).transpose()?;
if let Some(gd_const_value) = &gd_const_value {
gd_const_value.validate_const_expr(const_name, self.table)?;
}
Ok((gd_const_name, gd_const_value))
}).collect::<Result<_, GDError>>()?;
self.builder.add_decl(Decl::new(DeclF::EnumDecl(decl::EnumDecl { name: Some(gd_name), clauses: gd_clauses }), decl.pos));
Expand Down
2 changes: 0 additions & 2 deletions src/compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ impl Compiler {
_ => {
let mut class_scope = OutsideOfClass;
let expr = self.frame(pipeline, &mut (), table, &mut class_scope).compile_simple_expr("export", expr, NeedsResult::Yes)?;
expr.validate_const_expr("export", table)?;
Ok(expr)
}
}
Expand All @@ -143,7 +142,6 @@ impl Compiler {
// TODO Merge this with IRDecl::ConstDecl above
let gd_name = names::lisp_to_gd(&c.name);
let value = self.frame(pipeline, &mut (), table, class_scope).compile_simple_expr(&c.name, &c.value, NeedsResult::Yes)?;
value.validate_const_expr(&c.name, table)?;
Ok(Decl::new(DeclF::ConstDecl(gd_name, value), decl.pos))
}
ir::decl::ClassInnerDeclF::ClassVarDecl(v) => {
Expand Down
26 changes: 14 additions & 12 deletions src/ir/scope/decl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ use super::error::ScopeError;
use crate::util::extract_err;
use crate::pipeline::source::SourceOffset;
use crate::ir::identifier::{Namespace, ClassNamespace};
use crate::ir::decl::{DeclF, TopLevel, ClassDecl};
use crate::ir::decl::{Decl, DeclF, TopLevel, ClassDecl};
use crate::ir::expr::{Expr, ExprF, LambdaClass};
use crate::ir::literal::Literal;
use crate::gdscript::library;
use crate::optimize::ir::expr_walker::walk_exprs_in_toplevel;
use crate::optimize::ir::expr_walker::walk_exprs_in_decl;

use std::hash::Hash;
use std::convert::Infallible;
Expand Down Expand Up @@ -148,7 +148,7 @@ pub fn get_all_decl_scopes<'a>(toplevel: &'a TopLevel) -> Vec<Box<dyn DeclScope<
}

// Any anonymous classes declared in the file
let err = on_each_lambda_class::<Infallible, _>(toplevel, |class| {
let err = on_each_lambda_class::<Infallible, _>(&toplevel.decls, |class| {
// *sigh* What an unfortunate copy. But I don't see a way to
// convince the borrow checker that this value isn't going to
// disappear. (TODO Yeah...)
Expand All @@ -171,16 +171,18 @@ pub fn check_all_decl_scopes(toplevel: &TopLevel) -> Result<(), ScopeError<Class
Ok(())
}

pub fn on_each_lambda_class<E, F>(toplevel: &TopLevel, mut block: F) -> Result<(), E>
pub fn on_each_lambda_class<E, F>(decls: &[Decl], mut block: F) -> Result<(), E>
where F : FnMut(&LambdaClass) -> Result<(), E> {
walk_exprs_in_toplevel(toplevel, |expr| {
if let ExprF::LambdaClass(cls) = &expr.value {
block(cls)?;
}
// Note: We don't use this value, so if this string ever appears
// in the output code, there's a problem.
Ok(Expr::literal(Literal::from("UNUSED STRING FROM on_each_lambda_class"), SourceOffset(0)))
})?;
for decl in decls {
walk_exprs_in_decl(decl, |expr| {
if let ExprF::LambdaClass(cls) = &expr.value {
block(cls)?;
}
// Note: We don't use this value, so if this string ever appears
// in the output code, there's a problem.
Ok(Expr::literal(Literal::from("UNUSED STRING FROM on_each_lambda_class"), SourceOffset(0)))
})?;
}
Ok(())
}

Expand Down

0 comments on commit ca4ae3c

Please sign in to comment.