Skip to content

Commit

Permalink
Implements /proc/final, piped into should not override
Browse files Browse the repository at this point in the history
As a part of this we need to make a struct when building proc defs, to
keep track of their flags AND their kind at the same time.

We then pass that down into a new flags var on the proc decl struct, and
we're golden

Only thing of note is I removed the is_private and is_protected vars
from proc declerations because they were totally unused. Spooky added em
a long time ago and I think it was for not much reason
  • Loading branch information
LemonInTheDark committed Oct 4, 2023
1 parent 6df1be0 commit 379b2c9
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 35 deletions.
15 changes: 14 additions & 1 deletion crates/dreamchecker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -750,8 +750,21 @@ impl<'o> AnalyzeObjectTree<'o> {
}
}

/// Gather and store set directives for the given proc using the provided code body
/// Gather and store set directives for the given proc using the provided code body and already existing flags
pub fn gather_settings(&mut self, proc: ProcRef<'o>, code: &'o [Spanned<Statement>]) {
let proc_location = proc.get().location;
// Need to extract OUR declaration, and not our parent's. so we do the stupid
if let Some(proc_type) = proc.ty().get().procs.get(proc.name()) {
if let Some(declaration) = &proc_type.declaration {
let proc_flags = declaration.flags;
if proc_flags.is_final() {
// lemon todo: this should run, but it doesn't appear to trigger an error like I'd want. needs looking into imo
if let Err(error) = self.must_not_override.insert(proc, true, proc_location) {
self.context.register_error(error);
}
}
}
}
for statement in code.iter() {
if let Statement::Setting { ref name, ref value, .. } = statement.elem {
if name == "SpacemanDMM_return_type" {
Expand Down
27 changes: 27 additions & 0 deletions crates/dreamchecker/tests/directive_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,25 @@ fn no_override() {
check_errors_match(code, NO_OVERRIDE_ERRORS);
}


#[test]
fn final_proc() {
let code = r##"
/mob/proc/final/test()
return
/mob/subtype/test()
return
"##.trim();
check_errors_match(code, NO_OVERRIDE_ERRORS);
}

pub const NO_OVERRIDE_DISABLE_ERRORS: &[(u32, u16, &str)] = &[
(5, 5, "/mob/subtype/proc/test sets SpacemanDMM_should_not_override false, but it cannot be disabled."),
(4, 18, "proc overrides parent, prohibited by /mob/proc/test"),
];


#[test]
fn no_override_disable() {
let code = r##"
Expand All @@ -80,6 +94,19 @@ fn no_override_disable() {
check_errors_match(code, NO_OVERRIDE_DISABLE_ERRORS);
}

#[test]
fn final_proc_intermix() {
let code = r##"
/mob/proc/final/test()
return
/mob/subtype/test()
set SpacemanDMM_should_not_override = 0
return
"##.trim();
check_errors_match(code, NO_OVERRIDE_DISABLE_ERRORS);
}

#[test]
fn can_be_redefined() {
let code = r##"
Expand Down
67 changes: 67 additions & 0 deletions crates/dreammaker/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,36 @@ impl fmt::Display for PropertyAccessKind {
}
}

/// Information about a proc declaration
///
/// Holds what sort of decl it was (did it use /proc or /verb), alongside a set of flags
/// That describe extra info pulled from the path
#[derive(Debug, Clone, PartialEq, Eq, Copy, Hash)]
pub struct ProcDeclBuilder {
pub kind: ProcDeclKind,
pub flags: ProcFlags,
}

impl ProcDeclBuilder {
pub fn new(kind: ProcDeclKind, flags: Option<ProcFlags>) -> ProcDeclBuilder {
ProcDeclBuilder { kind: kind, flags: flags.unwrap_or(ProcFlags::default()) }
}

pub fn kind(self) -> &'static str {
self.kind.name()
}

pub fn is_final(self) -> bool {
self.flags.is_final()
}
}

impl fmt::Display for ProcDeclBuilder {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
fmt.write_str(format!("{}{}", self.kind, self.flags).as_str())
}
}

/// The proc declaration kind, either `proc` or `verb`.
///
/// DM requires referencing proc paths to include whether the target is
Expand Down Expand Up @@ -328,6 +358,43 @@ impl fmt::Display for ProcDeclKind {
}
}

bitflags! {
#[derive(Default)]
pub struct ProcFlags: u8 {
// DM flags
const FINAL = 1 << 0;
}
}

impl ProcFlags {
pub fn from_name(name: &str) -> Option<ProcFlags> {
match name {
// DM flags
"final" => Some(ProcFlags::FINAL),
// Fallback
_ => None,
}
}

#[inline]
pub fn is_final(&self) -> bool {
self.contains(ProcFlags::FINAL)
}

pub fn to_vec(&self) -> Vec<&'static str> {
let mut v = Vec::new();
if self.is_final() { v.push("final"); }
v
}
}

impl fmt::Display for ProcFlags {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
let written_entries = self.to_vec().into_iter().fold("".to_string(), |acc, elem| format!("{}/{}", acc, elem));
fmt.write_str(written_entries.as_str())
}
}

#[derive(Debug, Clone, Copy, PartialEq)]
pub enum SettingMode {
/// As in `set name = "Use"`.
Expand Down
26 changes: 15 additions & 11 deletions crates/dreammaker/src/objtree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::fmt;
use indexmap::IndexMap;
use ahash::RandomState;

use super::ast::{Expression, VarType, VarTypeBuilder, VarSuffix, PathOp, Parameter, Block, ProcDeclKind, Ident};
use super::ast::{Expression, VarType, VarTypeBuilder, VarSuffix, PathOp, Parameter, Block, ProcDeclBuilder, ProcDeclKind, ProcFlags, Ident};
use super::constants::Constant;
use super::docs::DocCollection;
use super::{DMError, Location, Context, Severity};
Expand Down Expand Up @@ -75,9 +75,8 @@ pub struct TypeVar {
pub struct ProcDeclaration {
pub location: Location,
pub kind: ProcDeclKind,
pub flags: ProcFlags,
pub id: SymbolId,
pub is_private: bool,
pub is_protected: bool,
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -1108,7 +1107,7 @@ impl ObjectTreeBuilder {
location: Location,
parent: NodeIndex,
name: &str,
declaration: Option<ProcDeclKind>,
declaration: Option<ProcDeclBuilder>,
parameters: Vec<Parameter>,
code: Option<Block>,
) -> Result<(usize, &mut ProcValue), DMError> {
Expand All @@ -1117,18 +1116,17 @@ impl ObjectTreeBuilder {
value: Vec::with_capacity(1),
declaration: None,
});
if let Some(kind) = declaration {
if let Some(decl_builder) = declaration {
if let Some(ref decl) = proc.declaration {
DMError::new(location, format!("duplicate definition of {}/{}", kind, name))
DMError::new(location, format!("duplicate definition of {}/{}", decl_builder.kind, name))
.with_note(decl.location, "previous definition")
.register(context);
} else {
proc.declaration = Some(ProcDeclaration {
location,
kind,
kind: decl_builder.kind,
flags: decl_builder.flags,
id: self.symbols.allocate(),
is_private: false,
is_protected: false,
});
}
}
Expand Down Expand Up @@ -1241,8 +1239,14 @@ impl ObjectTreeBuilder {
let (parent, mut proc_name) = self.get_from_path(location, &mut path, len)?;
let mut declaration = None;
if let Some(kind) = ProcDeclKind::from_name(proc_name) {
declaration = Some(kind);
proc_name = match path.next() {
let mut next_entry = path.next();
let flags = ProcFlags::from_name(next_entry.unwrap_or(""));
if let Some(_) = flags {
// did something? take another step
next_entry = path.next();
}
declaration = Some(ProcDeclBuilder::new(kind, flags));
proc_name = match next_entry {
Some(name) => name,
None => return Err(DMError::new(location, "proc must have a name")),
};
Expand Down
53 changes: 30 additions & 23 deletions crates/dreammaker/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,13 +588,13 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
self.tree_entries(self.tree.root_index(), None, None, Token::Eof)
}

fn tree_block(&mut self, current: NodeIndex, proc_kind: Option<ProcDeclKind>, var_type: Option<VarTypeBuilder>) -> Status<()> {
fn tree_block(&mut self, current: NodeIndex, proc_builder: Option<ProcDeclBuilder>, var_type: Option<VarTypeBuilder>) -> Status<()> {
leading!(self.exact(Token::Punct(Punctuation::LBrace)));
require!(self.tree_entries(current, proc_kind, var_type, Token::Punct(Punctuation::RBrace)));
require!(self.tree_entries(current, proc_builder, var_type, Token::Punct(Punctuation::RBrace)));
SUCCESS
}

fn tree_entries(&mut self, current: NodeIndex, proc_kind: Option<ProcDeclKind>, var_type: Option<VarTypeBuilder>, terminator: Token) -> Status<()> {
fn tree_entries(&mut self, current: NodeIndex, proc_builder: Option<ProcDeclBuilder>, var_type: Option<VarTypeBuilder>, terminator: Token) -> Status<()> {
loop {
let message: Cow<'static, str> = match terminator {
Token::Eof => "newline".into(),
Expand All @@ -607,7 +607,7 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
continue;
}
self.put_back(next);
require!(self.tree_entry(current, proc_kind, var_type.clone()));
require!(self.tree_entry(current, proc_builder, var_type.clone()));
}
SUCCESS
}
Expand Down Expand Up @@ -692,7 +692,7 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
}
}

fn tree_entry(&mut self, mut current: NodeIndex, mut proc_kind: Option<ProcDeclKind>, mut var_type: Option<VarTypeBuilder>) -> Status<()> {
fn tree_entry(&mut self, mut current: NodeIndex, mut proc_builder: Option<ProcDeclBuilder>, mut var_type: Option<VarTypeBuilder>) -> Status<()> {
// tree_entry :: path ';'
// tree_entry :: path tree_block
// tree_entry :: path '=' expression ';'
Expand Down Expand Up @@ -737,10 +737,16 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
var_type.type_path.push(each.to_owned());
}
} else if let Some(kind) = ProcDeclKind::from_name(each) {
proc_kind = Some(kind);
} else if proc_kind.is_some() {
self.error("cannot have sub-blocks of `proc/` block")
.register(self.context);
proc_builder = Some(ProcDeclBuilder::new(kind, None));
} else if let Some(builder) = proc_builder.as_mut() {
let flags = ProcFlags::from_name(each);
if let Some(found) = flags {
builder.flags |= found
}
else {
self.error("cannot have sub-blocks of `proc/` block")
.register(self.context);
}
} else {
let len = self.tree.get_path(current).chars().filter(|&c| c == '/').count() + path_len;
current = self.tree.subtype_or_add(self.location, current, each, len);
Expand Down Expand Up @@ -785,16 +791,16 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
handle_relative_type_error!();
let start = self.updated_location();

if proc_kind.is_some() || var_type.is_some() {
if proc_builder.is_some() || var_type.is_some() {
// Can't apply docs to `var/` or `proc/` blocks.
require!(self.tree_block(current, proc_kind, var_type.clone()));
require!(self.tree_block(current, proc_builder, var_type.clone()));
} else {
let (comment, ()) = require!(self.doc_comment(|this| this.tree_block(current, proc_kind, var_type.clone())));
let (comment, ()) = require!(self.doc_comment(|this| this.tree_block(current, proc_builder, var_type.clone())));
self.tree.extend_docs(current, comment);
}

let node = self.tree.get_path(current).to_owned();
self.annotate(start, || Annotation::TreeBlock(reconstruct_path(&node, proc_kind, var_type.as_ref(), "")));
self.annotate(start, || Annotation::TreeBlock(reconstruct_path(&node, proc_builder, var_type.as_ref(), "")));
SUCCESS
}
Punct(Assign) => {
Expand All @@ -811,7 +817,7 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
// We have to annotate prior to consuming the statement terminator, as we
// will otherwise consume following whitespace resulting in a bad annotation range
let node = this.tree.get_path(current).to_owned();
this.annotate(entry_start, || Annotation::Variable(reconstruct_path(&node, proc_kind, var_type.as_ref(), last_part)));
this.annotate(entry_start, || Annotation::Variable(reconstruct_path(&node, proc_builder, var_type.as_ref(), last_part)));

require!(this.statement_terminator());
success(expr)
Expand All @@ -829,7 +835,7 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
t @ Punct(LParen) => {
// `something(` - proc
self.put_back(t);
require!(self.proc_params_and_body(current, proc_kind, last_part, entry_start, absolute));
require!(self.proc_params_and_body(current, proc_builder, last_part, entry_start, absolute));
SUCCESS
}
other => {
Expand All @@ -850,14 +856,14 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
let docs = std::mem::take(&mut self.docs_following);
var_type.suffix(&var_suffix);
let node = self.tree.get_path(current).to_owned();
self.annotate(entry_start, || Annotation::Variable(reconstruct_path(&node, proc_kind, Some(&var_type), last_part)));
self.annotate(entry_start, || Annotation::Variable(reconstruct_path(&node, proc_builder, Some(&var_type), last_part)));
self.tree.declare_var(current, last_part, self.location, docs, var_type.build(), var_suffix.into_initializer());
}
} else if ProcDeclKind::from_name(last_part).is_some() {
self.error("`proc;` item has no effect")
.set_severity(Severity::Warning)
.register(self.context);
} else if proc_kind.is_some() {
} else if proc_builder.is_some() {
self.error("child of `proc/` without body")
.register(self.context);
} else {
Expand Down Expand Up @@ -948,7 +954,7 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
SUCCESS
}

fn proc_params_and_body(&mut self, current: NodeIndex, proc_kind: Option<ProcDeclKind>, name: &str, entry_start: Location, absolute: bool) -> Status<()> {
fn proc_params_and_body(&mut self, current: NodeIndex, proc_builder: Option<ProcDeclBuilder>, name: &str, entry_start: Location, absolute: bool) -> Status<()> {
use super::lexer::Token::*;
use super::lexer::Punctuation::*;

Expand Down Expand Up @@ -1016,12 +1022,12 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
None
};

match self.tree.register_proc(self.context, location, current, name, proc_kind, parameters, code) {
match self.tree.register_proc(self.context, location, current, name, proc_builder, parameters, code) {
Ok((idx, proc)) => {
proc.docs.extend(comment);
// manually performed for borrowck reasons
if let Some(dest) = self.annotations.as_mut() {
let new_stack = reconstruct_path(self.tree.get_path(current), proc_kind, None, name);
let new_stack = reconstruct_path(self.tree.get_path(current), proc_builder, None, name);
dest.insert(entry_start..body_start, Annotation::ProcHeader(new_stack.to_vec(), idx));
dest.insert(body_start..self.location, Annotation::ProcBody(new_stack.to_vec(), idx));
}
Expand Down Expand Up @@ -2370,13 +2376,14 @@ impl<'ctx, 'an, 'inp> Parser<'ctx, 'an, 'inp> {
}
}

fn reconstruct_path(node: &str, proc_kind: Option<ProcDeclKind>, var_type: Option<&VarTypeBuilder>, last: &str) -> Vec<Ident> {
fn reconstruct_path(node: &str, proc_deets: Option<ProcDeclBuilder>, var_type: Option<&VarTypeBuilder>, last: &str) -> Vec<Ident> {
let mut result = Vec::new();
for entry in node.split('/').skip(1) {
result.push(entry.to_owned());
}
if let Some(kind) = proc_kind {
result.push(kind.to_string());
if let Some(deets) = proc_deets {
result.push(deets.kind.to_string());
deets.flags.to_vec().into_iter().for_each(|elem| result.push(elem.to_string()));
}
if let Some(var) = var_type {
result.extend(var.flags.to_vec().into_iter().map(ToOwned::to_owned));
Expand Down

0 comments on commit 379b2c9

Please sign in to comment.