From 666b3d312f65334be41e6ef7471c705af3090e89 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 11 Aug 2023 15:57:13 -0400 Subject: [PATCH] tamer: asg::air: Generalize doc parsing This creates a new `AirDocAggregate` child parser and handled documentation in a consistent way for all object types. The logic that was previously object-specific now lives on the edge hooks that did not exist when doc parsing was originally introduced. This prepares for doc refs so that metavariables can be used for doc generation (most notably for interpolated documentation strings in templates). This also introduces a new error type specifically for meta, giving guidance to use ``. I don't like that it builds in source language assumptions in the help message provided, however I'd rather it be able to provide some guidance to people, especially given that the XML source language is the only one recognized by TAMER at the moment. DEV-13163 --- tamer/src/asg/air.rs | 136 ++++++++++++++++++++++++++--- tamer/src/asg/air/doc.rs | 104 ++++++++++++++++++++++ tamer/src/asg/air/expr.rs | 16 +--- tamer/src/asg/air/ir.rs | 10 +-- tamer/src/asg/air/meta.rs | 28 ++---- tamer/src/asg/air/pkg.rs | 39 ++------- tamer/src/asg/air/tpl.rs | 16 +--- tamer/src/asg/error.rs | 68 +++++++++++++-- tamer/src/asg/graph.rs | 6 +- tamer/src/asg/graph/object.rs | 18 ---- tamer/src/asg/graph/object/expr.rs | 33 ++++++- tamer/src/asg/graph/object/meta.rs | 30 ++++++- tamer/src/asg/graph/object/rel.rs | 54 +++++++++--- tamer/src/asg/graph/object/tpl.rs | 11 --- 14 files changed, 418 insertions(+), 151 deletions(-) create mode 100644 tamer/src/asg/air/doc.rs diff --git a/tamer/src/asg/air.rs b/tamer/src/asg/air.rs index 7a670b93..97f8fc8a 100644 --- a/tamer/src/asg/air.rs +++ b/tamer/src/asg/air.rs @@ -28,7 +28,7 @@ use super::{ graph::object::{ - Object, ObjectIndexRelTo, ObjectIndexTo, ObjectIndexToTree, + Doc, Object, ObjectIndexRelTo, ObjectIndexTo, ObjectIndexToTree, ObjectRelTy, ObjectRelatable, Pkg, Root, Tpl, }, Asg, AsgError, Expr, Ident, ObjectIndex, @@ -54,11 +54,13 @@ mod ir; use fxhash::FxHashMap; pub use ir::Air; +mod doc; mod expr; mod meta; mod opaque; mod pkg; mod tpl; +use doc::AirDocAggregate; use expr::AirExprAggregate; use meta::AirMetaAggregate; use opaque::AirOpaqueAggregate; @@ -107,6 +109,9 @@ pub enum AirAggregate { /// This parser is intended for loading declarations from object files /// without loading their corresponding definitions. PkgOpaque(AirOpaqueAggregate), + + /// Parsing documentation for active object. + PkgDoc(AirDocAggregate), } impl Display for AirAggregate { @@ -131,6 +136,9 @@ impl Display for AirAggregate { PkgOpaque(opaque) => { write!(f, "loading opaque objects: {opaque}") } + PkgDoc(doc) => { + write!(f, "parsing documentation: {doc}") + } } } } @@ -165,6 +173,12 @@ impl From for AirAggregate { } } +impl From for AirAggregate { + fn from(st: AirDocAggregate) -> Self { + Self::PkgDoc(st) + } +} + impl ParseState for AirAggregate { type Token = Air; type Object = (); @@ -201,11 +215,10 @@ impl ParseState for AirAggregate { // parent package frame. ( st @ (Root(..) | PkgExpr(..) | PkgTpl(..) | PkgMeta(..) - | PkgOpaque(..)), + | PkgOpaque(..) | PkgDoc(..)), tok @ AirPkg(..), ) => ctx.ret_or_transfer(st, tok, AirPkgAggregate::new()), (Pkg(pkg), AirPkg(etok)) => ctx.proxy(pkg, etok), - (Pkg(pkg), AirDoc(etok)) => ctx.proxy(pkg, etok), (st @ Pkg(..), tok @ AirBind(_)) => { ctx.try_ret_with_lookahead(st, tok) } @@ -216,7 +229,6 @@ impl ParseState for AirAggregate { } (PkgExpr(expr), AirExpr(etok)) => ctx.proxy(expr, etok), (PkgExpr(expr), AirBind(etok)) => ctx.proxy(expr, etok), - (PkgExpr(expr), AirDoc(etok)) => ctx.proxy(expr, etok), // Template (st @ (Pkg(_) | PkgExpr(_)), tok @ AirTpl(..)) => { @@ -224,7 +236,6 @@ impl ParseState for AirAggregate { } (PkgTpl(tplst), AirTpl(ttok)) => ctx.proxy(tplst, ttok), (PkgTpl(tplst), AirBind(ttok)) => ctx.proxy(tplst, ttok), - (PkgTpl(tplst), AirDoc(ttok)) => ctx.proxy(tplst, ttok), // Metavariables (st @ (PkgTpl(_) | PkgExpr(_)), tok @ AirMeta(..)) => { @@ -232,11 +243,30 @@ impl ParseState for AirAggregate { } (PkgMeta(meta), AirMeta(mtok)) => ctx.proxy(meta, mtok), (PkgMeta(meta), AirBind(mtok)) => ctx.proxy(meta, mtok), - (PkgMeta(meta), AirDoc(mtok)) => ctx.proxy(meta, mtok), (PkgMeta(meta), tok @ (AirExpr(..) | AirTpl(..))) => { ctx.try_ret_with_lookahead(meta, tok) } + // Documentation + // + // We let the child parser discover the active object rather + // than trying to discover it here; + // this ensures that child parses can be properly returned + // from if they have completed their task, + // otherwise documentation would be attached to the wrong + // object. + ( + st @ (Pkg(_) | PkgTpl(_) | PkgExpr(_) | PkgMeta(_)), + tok @ AirDoc(..), + ) => ctx.ret_or_transfer(st, tok, AirDocAggregate::new()), + (PkgDoc(doc), AirDoc(dtok)) => ctx.proxy(doc, dtok), + // Return from the doc parser for siblings; + // documentation strings have no children. + ( + PkgDoc(doc), + tok @ (AirBind(..) | AirExpr(..) | AirTpl(..) | AirMeta(..)), + ) => ctx.try_ret_with_lookahead(doc, tok), + // Opaque // // By having opaque object loading be its _own_ child parser, @@ -281,7 +311,8 @@ impl ParseState for AirAggregate { } ( - st @ (Root(..) | PkgExpr(..) | PkgTpl(..) | PkgMeta(..)), + st @ (Root(..) | PkgExpr(..) | PkgTpl(..) | PkgMeta(..) + | PkgDoc(..)), AirIdent(tok), ) => { Transition(st).err(AsgError::UnexpectedOpaqueIdent(tok.name())) @@ -296,6 +327,29 @@ impl ParseState for AirAggregate { } impl AirAggregate { + fn active_object(&self) -> Option> { + use AirAggregate::*; + + match self { + Uninit => None, + // The root is never an active object, + // since it can't be directly operated on. + Root(_) => None, + + Pkg(pkgst) => pkgst.active_pkg_oi().map(|oi| oi.widen()), + PkgExpr(exprst) => exprst.active_expr_oi().map(|oi| oi.widen()), + PkgTpl(tplst) => tplst.active_tpl_oi().map(|oi| oi.widen()), + PkgMeta(metast) => metast.active_meta_oi().map(|oi| oi.widen()), + + // Opaque objects are not intended to be manipulated. + PkgOpaque(_) => None, + + // Documentation objects are never active and are never intended + // to be the target of any operation. + PkgDoc(_) => None, + } + } + /// Whether the active parser is completed with active parsing. /// /// This method is used to determine whether control ought to be @@ -318,6 +372,7 @@ impl AirAggregate { PkgTpl(st) => st.is_accepting(ctx), PkgMeta(st) => st.is_accepting(ctx), PkgOpaque(st) => st.is_accepting(ctx), + PkgDoc(st) => st.is_accepting(ctx), } } @@ -337,6 +392,7 @@ impl AirAggregate { PkgTpl(st) => st.is_accepting(ctx), PkgMeta(st) => st.is_accepting(ctx), PkgOpaque(st) => st.is_accepting(ctx), + PkgDoc(st) => st.is_accepting(ctx), } } @@ -383,6 +439,9 @@ impl AirAggregate { // At the time of writing, // that is only a package. PkgOpaque(_) => None, + + // Documentation strings are not containers. + PkgDoc(_) => None, } } @@ -402,6 +461,26 @@ impl AirAggregate { } } + /// Active object that is able to be documented. + /// + /// Note that some objects only support certain types of documentation + /// and may produce an error when attempting to add an edge. + fn active_doc_oi(&self) -> Option> { + use AirAggregate::*; + + // We consider only the topmost stack item. + match self { + Uninit => None, + Root(_) => None, + Pkg(pkgst) => pkgst.active_pkg_oi().map(Into::into), + PkgExpr(exprst) => exprst.active_expr_oi().map(Into::into), + PkgTpl(tplst) => tplst.active_tpl_oi().map(Into::into), + PkgMeta(metast) => metast.active_meta_oi().map(Into::into), + PkgOpaque(_) => None, + PkgDoc(_) => None, + } + } + /// The boundary associated with the active environment. /// /// If the active parser does not introduce its own scope @@ -417,10 +496,11 @@ impl AirAggregate { Uninit => Transparent, PkgOpaque(_) => Transparent, - // Expressions and metadata are not containers, + // These are not containers, // and do not introduce scope. PkgExpr(_) => Transparent, PkgMeta(_) => Transparent, + PkgDoc(_) => Transparent, // Packages and templates act as containers and so restrict // identifier scope. @@ -467,6 +547,9 @@ impl AirAggregate { // If an object is opaque to us then we cannot possibly look // into it to see what needs expansion. PkgOpaque(_) => false, + + // Documentation strings cannot own identifiers. + PkgDoc(_) => false, } } } @@ -745,7 +828,7 @@ impl AirAggregateCtx { eoi: EnvScopeKind>, ) -> Result<(), ObjectIndex> { let sym = name.into(); - let ient = index.entry((O::rel_ty(), sym, imm_env.widen())); + let ient = index.entry((O::rel_ty(), sym, imm_env.widen_src())); use Entry::*; use EnvScopeKind::*; @@ -811,7 +894,7 @@ impl AirAggregateCtx { use crate::fmt::{DisplayWrapper, TtQuote}; crate::debug_diagnostic_panic!( vec![ - imm_env.widen().note("at this scope boundary"), + imm_env.widen_src().note("at this scope boundary"), prev_oi.note("previously indexed identifier was here"), eoi.internal_error( "this identifier has already been indexed at the above scope boundary" @@ -956,9 +1039,31 @@ impl AirAggregateCtx { // not opaque, // and so not permitted in this context. PkgOpaque(_) => None, + + // Documentation cannot contain expressions. + PkgDoc(_) => None, }) } + /// Require that the object atop of the stack be documentable, + /// otherwise producing an error. + /// + /// See also [`AirAggregate::active_doc_oi`]. + fn require_active_doc_oi( + &self, + at: Span, + ) -> Result, AsgError> { + // We consider only the topmost stack frame. + let ost = self.stack.iter().rev().next(); + + ost.and_then(|st| st.active_doc_oi()).ok_or( + AsgError::InvalidDocContext( + at, + ost.and_then(|st| st.active_object()).map(|oi| oi.span()), + ), + ) + } + /// The active expansion target (splicing context) for [`Tpl`]s. /// /// A value of [`None`] indicates that template expansion is not @@ -984,6 +1089,9 @@ impl AirAggregateCtx { // and so it'd be best to leave this alone unless it's // actually needed. PkgOpaque(_) => None, + + // Documentation strings are not containers. + PkgDoc(_) => None, }) } @@ -1035,9 +1143,9 @@ impl AirAggregateCtx { // If it's a problem, // we can have `instantiable_rooting_oi` retain // information. - let rooting_span = self - .rooting_oi() - .map(|(_, oi)| oi.widen().resolve(self.asg_ref()).span()); + let rooting_span = self.rooting_oi().map(|(_, oi)| { + oi.widen_src().resolve(self.asg_ref()).span() + }); // Note that we _discard_ the attempted bind token // and so remain in a dangling state. @@ -1143,7 +1251,7 @@ impl AirAggregateCtx { // Maybe future Rust will have dependent types that allow for better // static assurances. self.index - .get(&(O::rel_ty(), id.symbol(), imm_env.widen())) + .get(&(O::rel_ty(), id.symbol(), imm_env.widen_src())) .map(|&eoi| { eoi.map(|oi| oi.overwrite(id.span()).must_narrow_into::()) }) diff --git a/tamer/src/asg/air/doc.rs b/tamer/src/asg/air/doc.rs new file mode 100644 index 00000000..5916822f --- /dev/null +++ b/tamer/src/asg/air/doc.rs @@ -0,0 +1,104 @@ +// ASG IR documentation parsing +// +// Copyright (C) 2014-2023 Ryan Specialty, LLC. +// +// This file is part of TAME. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +//! AIR doc parser. +//! +//! See the [parent module](super) for more information. + +use super::{super::AsgError, ir::AirDoc, AirAggregate, AirAggregateCtx}; +use crate::{asg::graph::object::ObjectIndexTreeRelTo, parse::prelude::*}; + +/// Package parsing with support for loaded identifiers. +/// +/// This supports non-nested package definitions of source files, +/// as well as declaring opaque identifiers loaded from object files via +/// [`AirIdent`](super::ir::AirIdent). +#[derive(Debug, PartialEq)] +pub enum AirDocAggregate { + /// Ready to document active object. + Ready, +} + +impl Display for AirDocAggregate { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + use AirDocAggregate::*; + + match self { + Ready => { + write!(f, "expecting documentation for object") + } + } + } +} + +impl ParseState for AirDocAggregate { + type Token = AirDoc; + type Object = (); + type Error = AsgError; + type Context = AirAggregateCtx; + type Super = AirAggregate; + + fn parse_token( + self, + tok: Self::Token, + ctx: &mut Self::Context, + ) -> TransitionResult { + use AirDoc::*; + use AirDocAggregate::*; + + match (self, tok) { + (Ready, DocIndepClause(clause)) => ctx + .require_active_doc_oi(clause.span()) + .and_then(|oi| oi.add_desc_short(ctx.asg_mut(), clause)) + .map(|_| ()) + .transition(Ready), + + (Ready, DocIndepClauseRef(name)) => { + let oi_ident = ctx.lookup_lexical_or_missing(name); + + ctx.require_active_doc_oi(name.span()) + .and_then(|oi| { + oi.add_desc_short_ref( + ctx.asg_mut(), + oi_ident, + name.span(), + ) + }) + .map(|_| ()) + .transition(Ready) + } + + (Ready, DocText(text)) => ctx + .require_active_doc_oi(text.span()) + .and_then(|oi| oi.append_doc_text(ctx.asg_mut(), text)) + .map(|_| ()) + .transition(Ready), + } + } + + fn is_accepting(&self, _: &Self::Context) -> bool { + true + } +} + +impl AirDocAggregate { + pub fn new() -> Self { + Self::Ready + } +} diff --git a/tamer/src/asg/air/expr.rs b/tamer/src/asg/air/expr.rs index 4c87279b..57e2ebce 100644 --- a/tamer/src/asg/air/expr.rs +++ b/tamer/src/asg/air/expr.rs @@ -81,7 +81,7 @@ impl ParseState for AirExprAggregate { tok: Self::Token, ctx: &mut Self::Context, ) -> crate::parse::TransitionResult { - use super::ir::{AirBind::*, AirDoc::*, AirExpr::*}; + use super::ir::{AirBind::*, AirExpr::*}; use AirBindableExpr::*; use AirExprAggregate::*; @@ -165,24 +165,12 @@ impl ParseState for AirExprAggregate { .transition(BuildingExpr(es, oi)) } - (BuildingExpr(es, oi), AirDoc(DocIndepClause(clause))) => oi - .add_desc_short(ctx.asg_mut(), clause) - .map(|_| ()) - .transition(BuildingExpr(es, oi)), - - (BuildingExpr(es, oi), AirDoc(DocText(text))) => Transition( - BuildingExpr(es, oi), - ) - .err(AsgError::InvalidDocContextExpr(oi.span(), text.span())), - (st @ Ready(..), AirExpr(ExprEnd(span))) => { Transition(st).err(AsgError::UnbalancedExpr(span)) } // Token may refer to a parent context. - (st @ Ready(..), tok @ (AirBind(..) | AirDoc(..))) => { - Transition(st).dead(tok) - } + (st @ Ready(..), tok @ AirBind(..)) => Transition(st).dead(tok), } } diff --git a/tamer/src/asg/air/ir.rs b/tamer/src/asg/air/ir.rs index 3361e341..d6bb9f9b 100644 --- a/tamer/src/asg/air/ir.rs +++ b/tamer/src/asg/air/ir.rs @@ -876,21 +876,17 @@ sum_ir! { } } - /// Package definitions interspersed with documentation in a - /// literate style. - pub sum enum AirLiteratePkg = AirPkg | AirDoc; - /// Expressions that are able to be bound to identifiers. /// /// This is the primary token set when parsing packages, /// since most everything in TAMER is an expression. - pub sum enum AirBindableExpr = AirExpr | AirBind | AirDoc; + pub sum enum AirBindableExpr = AirExpr | AirBind; /// Tokens that may be used to define or apply templates. - pub sum enum AirBindableTpl = AirTpl | AirBind | AirDoc; + pub sum enum AirBindableTpl = AirTpl | AirBind; /// Tokens that may be used to define metavariables. - pub sum enum AirBindableMeta = AirMeta | AirBind | AirDoc; + pub sum enum AirBindableMeta = AirMeta | AirBind; } impl AirIdent { diff --git a/tamer/src/asg/air/meta.rs b/tamer/src/asg/air/meta.rs index 6815b0b8..a83e7870 100644 --- a/tamer/src/asg/air/meta.rs +++ b/tamer/src/asg/air/meta.rs @@ -61,7 +61,7 @@ impl ParseState for AirMetaAggregate { tok: Self::Token, ctx: &mut Self::Context, ) -> TransitionResult { - use super::ir::{AirBind::*, AirDoc::*, AirMeta::*}; + use super::ir::{AirBind::*, AirMeta::*}; use AirBindableMeta::*; use AirMetaAggregate::*; @@ -101,21 +101,6 @@ impl ParseState for AirMetaAggregate { ) } - (TplMeta(oi_meta), AirDoc(DocIndepClause(clause))) => oi_meta - .add_desc_short(ctx.asg_mut(), clause) - .map(|_| ()) - .transition(TplMeta(oi_meta)), - - // TODO: The user _probably_ meant to use `` in XML NIR, - // so maybe we should have an error to that effect. - (TplMeta(..), tok @ AirDoc(DocText(..))) => { - diagnostic_todo!( - vec![tok.note("this token")], - "AirDoc in metavar context \ - (is this something we want to support?)" - ) - } - // Reference to another metavariable, // e.g. using `` in XML NIR. (TplMeta(oi_meta), AirBind(RefIdent(name))) => { @@ -149,9 +134,7 @@ impl ParseState for AirMetaAggregate { } // Maybe the token can be handled by the parent frame. - (Ready, tok @ (AirBind(..) | AirDoc(..))) => { - Transition(Ready).dead(tok) - } + (Ready, tok @ AirBind(..)) => Transition(Ready).dead(tok), } } @@ -164,6 +147,13 @@ impl AirMetaAggregate { pub(super) fn new() -> Self { Self::Ready } + + pub fn active_meta_oi(&self) -> Option> { + match self { + AirMetaAggregate::Ready => None, + AirMetaAggregate::TplMeta(oi) => Some(*oi), + } + } } #[cfg(test)] diff --git a/tamer/src/asg/air/pkg.rs b/tamer/src/asg/air/pkg.rs index 714a6e5f..8245144d 100644 --- a/tamer/src/asg/air/pkg.rs +++ b/tamer/src/asg/air/pkg.rs @@ -23,10 +23,10 @@ use super::{ super::{graph::object::Pkg, AsgError, ObjectIndex}, - ir::AirLiteratePkg, + ir::AirPkg, AirAggregate, AirAggregateCtx, }; -use crate::{diagnostic_todo, parse::prelude::*}; +use crate::parse::prelude::*; /// Package parsing with support for loaded identifiers. /// @@ -59,7 +59,7 @@ impl Display for AirPkgAggregate { } impl ParseState for AirPkgAggregate { - type Token = AirLiteratePkg; + type Token = AirPkg; type Object = (); type Error = AsgError; type Context = AirAggregateCtx; @@ -70,12 +70,11 @@ impl ParseState for AirPkgAggregate { tok: Self::Token, ctx: &mut Self::Context, ) -> crate::parse::TransitionResult { - use super::ir::{AirDoc::*, AirPkg::*}; - use AirLiteratePkg::*; + use AirPkg::*; use AirPkgAggregate::*; match (self, tok) { - (st @ (Ready | Toplevel(..)), AirPkg(PkgStart(span, name))) => { + (st @ (Ready | Toplevel(..)), PkgStart(span, name)) => { if let Some(first) = ctx.pkg_oi().map(|oi| oi.resolve(ctx.asg_ref())) { @@ -94,45 +93,25 @@ impl ParseState for AirPkgAggregate { } } - (Toplevel(oi_pkg), AirPkg(PkgEnd(span))) => { + (Toplevel(oi_pkg), PkgEnd(span)) => { oi_pkg.close(ctx.asg_mut(), span); ctx.pkg_clear(); Transition(Ready).incomplete() } - (Toplevel(oi_pkg), tok @ AirDoc(DocIndepClause(..))) => { - diagnostic_todo!( - vec![ - oi_pkg.note("for this package"), - tok.internal_error( - "this package description is not yet supported" - ) - ], - "package-level short description is not yet supported by TAMER", - ) - } - - (Toplevel(oi_pkg), AirDoc(DocText(text))) => oi_pkg - .append_doc_text(ctx.asg_mut(), text) - .map(|_| ()) - .transition(Toplevel(oi_pkg)), - // Package import - (Toplevel(oi_pkg), AirPkg(PkgImport(namespec))) => oi_pkg + (Toplevel(oi_pkg), PkgImport(namespec)) => oi_pkg .import(ctx.asg_mut(), namespec) .map(|_| ()) .transition(Toplevel(oi_pkg)), - (Ready, AirPkg(PkgImport(namespec))) => { + (Ready, PkgImport(namespec)) => { Transition(Ready).err(AsgError::InvalidPkgImport(namespec)) } - (Ready, AirPkg(PkgEnd(span))) => { + (Ready, PkgEnd(span)) => { Transition(Ready).err(AsgError::InvalidPkgEndContext(span)) } - - // Token may refer to a parent context. - (st @ Ready, tok @ AirDoc(..)) => Transition(st).dead(tok), } } diff --git a/tamer/src/asg/air/tpl.rs b/tamer/src/asg/air/tpl.rs index 707778f1..997a37ba 100644 --- a/tamer/src/asg/air/tpl.rs +++ b/tamer/src/asg/air/tpl.rs @@ -163,7 +163,7 @@ impl ParseState for AirTplAggregate { tok: Self::Token, ctx: &mut Self::Context, ) -> TransitionResult { - use super::ir::{AirBind::*, AirDoc::*, AirTpl::*}; + use super::ir::{AirBind::*, AirTpl::*}; use AirBindableTpl::*; use AirTplAggregate::*; @@ -209,18 +209,6 @@ impl ParseState for AirTplAggregate { .transition(Toplevel(tpl)) } - (Toplevel(tpl), AirDoc(DocIndepClause(clause))) => tpl - .oi() - .add_desc_short(ctx.asg_mut(), clause) - .map(|_| ()) - .transition(Toplevel(tpl)), - - (Toplevel(tpl), AirDoc(DocText(text))) => tpl - .oi() - .append_doc_text(ctx.asg_mut(), text) - .map(|_| ()) - .transition(Toplevel(tpl)), - (Toplevel(tpl), AirTpl(TplEnd(span))) => { tpl.close(ctx.asg_mut(), span).map(|_| ()).transition(Done) } @@ -260,7 +248,7 @@ impl ParseState for AirTplAggregate { Transition(Ready).err(AsgError::UnbalancedTpl(span)) } - (st @ (Ready | Done), tok @ (AirBind(..) | AirDoc(..))) => { + (st @ (Ready | Done), tok @ AirBind(..)) => { Transition(st).dead(tok) } } diff --git a/tamer/src/asg/error.rs b/tamer/src/asg/error.rs index a8bd129d..95e7fc04 100644 --- a/tamer/src/asg/error.rs +++ b/tamer/src/asg/error.rs @@ -141,16 +141,27 @@ pub enum AsgError { /// expansion. InvalidExpansionContext(Span), + /// A short description cannot be applied to the active object. + /// + /// The error span is that of the description, + /// and the context span is the active object that does not support + /// that description, + /// if any. + InvalidDocContext(ErrorOccurrenceSpan, Option), + /// Documentation text is not valid in an expression context. /// /// This historical limitation existed because the author was unsure how /// to go about rendering an equation with literate documentation /// interspersed. /// The plan is to lift this limitation in the future. + InvalidDocContextExpr(ErrorOccurrenceSpan, ErrorContextSpan), + + /// Documentation text is not valid within a metavariable. /// - /// The spans represent the expression and the documentation text - /// respectively. - InvalidDocContextExpr(Span, Span), + /// The equivalent lexical value is permitted, + /// but it cannot be conveyed as documentation. + InvalidDocContextMeta(ErrorOccurrenceSpan, ErrorContextSpan), /// A circular dependency was found where it is not permitted. /// @@ -187,6 +198,12 @@ type ErrorOccurrenceSpan = Span; /// This should be paired with [`ErrorOccurrenceSpan`]. type FirstOccurrenceSpan = Span; +/// The context in which an error occurred. +/// +/// For example, +/// this may refer to the container of the object that caused an error. +type ErrorContextSpan = Span; + impl Display for AsgError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { use AsgError::*; @@ -252,9 +269,22 @@ impl Display for AsgError { InvalidExpansionContext(_) => { write!(f, "invalid template expansion context",) } - InvalidDocContextExpr(_, _) => { - write!(f, "document text is not permitted within expressions") + + InvalidDocContext(_, _) => { + write!(f, "documentation is not supported in this context") } + + InvalidDocContextExpr(_, _) => write!( + f, + "inline documentation is not permitted within expressions" + ), + + InvalidDocContextMeta(_, _) => write!( + f, + "inline documentation is not permitted within metavariables \ + (template parameters)" + ), + UnsupportedCycle(cycle) => { write!(f, "circular dependency: {cycle}") } @@ -427,14 +457,38 @@ impl Diagnostic for AsgError { vec![span.error("cannot expand a template here")] } - InvalidDocContextExpr(expr_span, span) => vec![ + InvalidDocContext(doc, octx) => vec![ + match octx { + Some(ctx) => { + ctx.note("attempting to document this active object") + } + None => doc.note("there is no active object to document"), + }, + doc.error( + "this documentation cannot be applied to the active object", + ), + ], + + InvalidDocContextExpr(span, expr_span) => vec![ expr_span.note("in this expression"), - span.error("documentation text is not permitted here"), + span.error("inline documentation is not permitted here"), span.help( "this is a historical limitation that will \ likely be lifted in the future", ), ], + + InvalidDocContextMeta(span, meta_span) => vec![ + meta_span.note("in this metavariable"), + span.error("inline documentation is not permitted here"), + // TODO: This is assuming that the source language is an + // XML NIR, + // because we don't at the time of writing yet have the + // ability to augment errors with additional information + // from other pipeline components. + span.help("did you mean to enclose this in a `` node?"), + ], + UnsupportedCycle(cycle) => { // The cycle description clearly describes the cycle, // but in neutral terms, diff --git a/tamer/src/asg/graph.rs b/tamer/src/asg/graph.rs index 920dc9ee..6327a059 100644 --- a/tamer/src/asg/graph.rs +++ b/tamer/src/asg/graph.rs @@ -217,7 +217,7 @@ impl Asg { ) -> Result<(), AsgError> { from_oi.pre_add_edge(self, to_oi, None, |asg| { asg.graph.add_edge( - from_oi.widen().into(), + from_oi.widen_src().into(), to_oi.into(), (from_oi.src_rel_ty(), OB::rel_ty(), None), ); @@ -255,7 +255,7 @@ impl Asg { ) -> Result<(), AsgError> { from_oi.pre_add_edge(self, to_oi, Some(ref_span), |asg| { asg.graph.add_edge( - from_oi.widen().into(), + from_oi.widen_src().into(), to_oi.into(), (from_oi.src_rel_ty(), OB::rel_ty(), Some(ref_span)), ); @@ -419,7 +419,7 @@ impl Asg { from: impl ObjectIndexRelTo, to: ObjectIndex, ) -> bool { - self.graph.contains_edge(from.widen().into(), to.into()) + self.graph.contains_edge(from.widen_src().into(), to.into()) } pub(super) fn expect_obj(&self, oi: ObjectIndex) -> &O { diff --git a/tamer/src/asg/graph/object.rs b/tamer/src/asg/graph/object.rs index 8506eff6..4fc169fe 100644 --- a/tamer/src/asg/graph/object.rs +++ b/tamer/src/asg/graph/object.rs @@ -966,24 +966,6 @@ impl ObjectIndex { oi.defined_by(asg, self).map(|_| self) } - /// Describe this expression using a short independent clause. - /// - /// This is intended to be a concise description for use either as a - /// simple sentence or as part of a compound sentence. - /// There should only be one such clause for any given object, - /// but that is not enforced here. - pub fn add_desc_short( - &self, - asg: &mut Asg, - clause: SPair, - ) -> Result - where - O: ObjectTreeRelTo, - { - let oi_doc = asg.create(Doc::new_indep_clause(clause)); - self.add_tree_edge_to(asg, oi_doc) - } - /// Retrieve a description of this expression using a short independent /// clause, /// if one has been set. diff --git a/tamer/src/asg/graph/object/expr.rs b/tamer/src/asg/graph/object/expr.rs index ced44fc8..42a5c623 100644 --- a/tamer/src/asg/graph/object/expr.rs +++ b/tamer/src/asg/graph/object/expr.rs @@ -75,8 +75,11 @@ use super::{ ident::IdentDefinition, prelude::*, Doc, Ident, ObjectIndexToTree, Tpl, }; use crate::{ - asg::graph::ProposedRel, diagnose::panic::DiagnosticPanic, num::Dim, - parse::prelude::Annotate, span::Span, + asg::graph::ProposedRel, + diagnose::panic::DiagnosticPanic, + num::Dim, + parse::{prelude::Annotate, Token}, + span::Span, }; use std::{fmt::Display, num::NonZeroU16}; @@ -500,7 +503,31 @@ object_rel! { } }, - tree Doc, + tree Doc { + fn pre_add_edge( + asg: &mut Asg, + rel: ProposedRel, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError> { + let doc = rel.to_oi.resolve(asg); + + match doc { + Doc::IndepClause(_) => Ok(commit(asg)), + + // This maintains compatibility with the XLST-based + // system. + // TODO: Inline documentation was prohibited within + // expressions because of questions of how it would + // render within the Summary Page; + // this restriction should just be lifted and dealt + // with separately. + Doc::Text(spair) => Err(AsgError::InvalidDocContextExpr( + spair.span(), + rel.from_oi.resolve(asg).span(), + )) + } + } + }, // Deferred template application tree Tpl, diff --git a/tamer/src/asg/graph/object/meta.rs b/tamer/src/asg/graph/object/meta.rs index 5eb55718..0199e066 100644 --- a/tamer/src/asg/graph/object/meta.rs +++ b/tamer/src/asg/graph/object/meta.rs @@ -34,6 +34,7 @@ use arrayvec::ArrayVec; use super::{prelude::*, Doc, Ident}; use crate::{ + asg::graph::ProposedRel, diagnose::Annotate, diagnostic_todo, f::Map, @@ -181,7 +182,34 @@ object_rel! { tree Meta, // e.g. template paramater description. - tree Doc, + tree Doc { + fn pre_add_edge( + asg: &mut Asg, + rel: ProposedRel, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError> { + let doc = rel.to_oi.resolve(asg); + + match doc { + Doc::IndepClause(_) => Ok(commit(asg)), + + // It doesn't make sense to allow inline documentation + // here, + // even though an equivalent lexical value would + // make sense; + // it's the expected interpretation that's the + // problem, + // not the lexical content. + // This isn't possible to hit at the time of writing + // when using XML->NIR because of the ambiguity of the + // `Nir::Text` token. + Doc::Text(spair) => Err(AsgError::InvalidDocContextMeta( + spair.span(), + rel.from_oi.resolve(asg).span(), + )) + } + } + }, } } diff --git a/tamer/src/asg/graph/object/rel.rs b/tamer/src/asg/graph/object/rel.rs index ef722a0a..d08cc4ce 100644 --- a/tamer/src/asg/graph/object/rel.rs +++ b/tamer/src/asg/graph/object/rel.rs @@ -31,6 +31,7 @@ use crate::{ Asg, AsgError, }, f::Map, + parse::util::SPair, span::Span, }; use std::{fmt::Display, marker::PhantomData}; @@ -866,7 +867,7 @@ pub trait ObjectIndexRelTo: Sized + Clone + Copy { /// [`ObjectKind`] information. /// /// See [`ObjectIndex::widen`] for more information. - fn widen(&self) -> ObjectIndex; + fn widen_src(&self) -> ObjectIndex; /// Request permission to add an edge from `self` to another object. /// @@ -910,7 +911,7 @@ pub trait ObjectIndexRelTo: Sized + Clone + Copy { &self, asg: &'a Asg, ) -> impl Iterator> + 'a { - asg.edges_dyn(self.widen()) + asg.edges_dyn(self.widen_src()) .filter_map(|rel| rel.filter_into_target()) } @@ -966,7 +967,7 @@ where O::rel_ty() } - fn widen(&self) -> ObjectIndex { + fn widen_src(&self) -> ObjectIndex { ObjectIndex::::widen(*self) } @@ -996,7 +997,7 @@ impl ObjectIndexRelTo for ObjectIndexTo { } } - fn widen(&self) -> ObjectIndex { + fn widen_src(&self) -> ObjectIndex { *self.as_ref() } @@ -1012,7 +1013,7 @@ impl ObjectIndexRelTo for ObjectIndexTo { $ty::pre_add_edge( asg, ProposedRel { - from_oi: self.widen().must_narrow_into::<$ty>(), + from_oi: self.widen_src().must_narrow_into::<$ty>(), to_oi, ref_span, }, @@ -1040,9 +1041,9 @@ impl ObjectIndexRelTo for ObjectIndexToTree { } } - fn widen(&self) -> ObjectIndex { + fn widen_src(&self) -> ObjectIndex { match self { - Self(oito) => oito.widen(), + Self(oito) => oito.widen_src(), } } @@ -1066,9 +1067,9 @@ impl ObjectIndexRelTo for ObjectIndexToCross { } } - fn widen(&self) -> ObjectIndex { + fn widen_src(&self) -> ObjectIndex { match self { - Self(oito) => oito.widen(), + Self(oito) => oito.widen_src(), } } @@ -1087,7 +1088,7 @@ impl ObjectIndexRelTo for ObjectIndexToCross { impl From> for ObjectIndex { fn from(oi_rel: ObjectIndexTo) -> Self { - oi_rel.widen() + oi_rel.widen_src() } } @@ -1110,6 +1111,39 @@ impl AsRef> for ObjectIndexTo { pub trait ObjectIndexTreeRelTo: ObjectIndexRelTo + Into> { + /// Describe this expression using a short independent clause. + /// + /// This is intended to be a concise description for use either as a + /// simple sentence or as part of a compound sentence. + /// There should only be one such clause for any given object, + /// but that is not enforced here. + fn add_desc_short( + self, + asg: &mut Asg, + clause: SPair, + ) -> Result + where + Self: ObjectIndexTreeRelTo, + { + let oi_doc = asg.create(Doc::new_indep_clause(clause)); + asg.add_tree_edge(self, oi_doc)?; + Ok(self) + } + + /// Arbitrary text serving as documentation in a literate style, + /// to be expanded into the application site. + fn append_doc_text( + self, + asg: &mut Asg, + text: SPair, + ) -> Result + where + Self: ObjectIndexTreeRelTo, + { + let oi_doc = asg.create(Doc::new_text(text)); + asg.add_tree_edge(self, oi_doc)?; + Ok(self) + } } impl ObjectIndexTreeRelTo for ObjectIndexToTree {} diff --git a/tamer/src/asg/graph/object/tpl.rs b/tamer/src/asg/graph/object/tpl.rs index 45f669a9..605a372e 100644 --- a/tamer/src/asg/graph/object/tpl.rs +++ b/tamer/src/asg/graph/object/tpl.rs @@ -415,15 +415,4 @@ impl ObjectIndex { ) -> Result { self.add_tree_edge_from(asg, oi_target_parent) } - - /// Arbitrary text serving as documentation in a literate style, - /// to be expanded into the application site. - pub fn append_doc_text( - &self, - asg: &mut Asg, - text: SPair, - ) -> Result { - let oi_doc = asg.create(Doc::new_text(text)); - self.add_tree_edge_to(asg, oi_doc) - } }