diff --git a/CHANGELOG.md b/CHANGELOG.md index bea6b8a..b72e915 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,8 @@ # Change Log ## 0.9.20 - +- The DLS will now properly not report conflicts between statements in a `#if` and its corresponding `#else` branch +- The DLS will now consider all `#if` branches on conditions directly based on `dml\_1\_2` and `dml\_1\_4` dead or alive appropriately ## 0.9.19 - Added configuration option to control the max cache size while resolving references in semantic analysis, defaulting to 500MB diff --git a/src/analysis/structure/toplevel.rs b/src/analysis/structure/toplevel.rs index 23a7fea..8f2b115 100644 --- a/src/analysis/structure/toplevel.rs +++ b/src/analysis/structure/toplevel.rs @@ -2,12 +2,14 @@ // SPDX-License-Identifier: Apache-2.0 and MIT use std::fmt::{Display, Formatter, self as fmt}; use std::path::PathBuf; +use std::sync::Arc; +use crate::analysis::templating::evaluation::{Evaluatable, EvaluationContext}; use crate::logging::trace; use crate::analysis::structure::objects::{Bitorder, CBlock, CompObjectKind, CompositeObject, Constant, DMLObject, DMLStatement, Device, Error, Export, Hook, Import, InEach, Instantiation, Loggroup, Method, MethodModifier, Parameter, Statements, Template, ToStructure, Typedef, Variable, Version, make_statements}; use crate::analysis::structure::expressions::{Expression}; -use crate::analysis::FileSpec; +use crate::analysis::{DMLError, FileSpec}; use crate::analysis::parsing::tree::{ZeroRange, ZeroSpan, TreeElement}; use crate::analysis::parsing::structure; use crate::analysis::{DeclarationSpan, LocationSpan, Named, @@ -30,7 +32,51 @@ pub enum ExistCondition { // Note: Right now we are implicitly assuming this has some ordering, such // that two objectdecls within the same hashif block will have the same // existcondition - Conditional(Vec<(bool, Expression)>), + // These conditions are _hugely_ duplicated between objects, and are stored + // in an Arc to save memory. TODO: We could optimize this even further by + // storing references into a slice of conditional expression from outer + // to inner contexts + Conditional(Arc>), +} + +impl ExistCondition { + pub fn exists(&self, context: &EvaluationContext, report: &mut Vec) -> bool { + match self { + ExistCondition::Always => true, + ExistCondition::Conditional(conds) => { + for (tbranch, cond) in conds.as_ref() { + let evaluated = cond.evaluate(context, report); + // TODO: check type and value validness + // NOTE: this concludes that non-evaluatable conditions are treated + // as always existing + if evaluated.as_bool() + .is_some_and(|b|b != *tbranch) { + return false; + } + } + true + } + } + } + pub fn guaranteed_exists(&self, context: &EvaluationContext, report: &mut Vec) -> bool { + match self { + ExistCondition::Always => true, + ExistCondition::Conditional(conds) => { + for (tbranch, cond) in conds.as_ref() { + let evaluated = cond.evaluate(context, report); + // TODO: check type and value validness + if let Some(value) = evaluated.as_bool() { + if value != *tbranch { + return false; + } + } else { + return false; + } + } + true + } + } + } } impl ExistCondition { @@ -39,10 +85,8 @@ impl ExistCondition { (ExistCondition::Always, ExistCondition::Always) => true, (ExistCondition::Conditional(selfvec), ExistCondition::Conditional(othervec)) => { - // This becomes equivalent to checking if they are in the - // same nested hashifs - // As it turns out, collision is guaranted regardless of - // which branch they are in + // Currently we cannt check if a condition is equivalent with another, + // so we will only check if they are literally the same condition expression for ((_, cond1), (_, cond2)) in selfvec.iter().zip(othervec.iter()) { if cond1 != cond2 { @@ -54,6 +98,27 @@ impl ExistCondition { _ => false, } } + + pub fn guaranteed_excluded_from(&self, other: &ExistCondition) -> bool { + match (self, other) { + (ExistCondition::Conditional(selfvec), + ExistCondition::Conditional(othervec)) => { + // Currently we cannt check if a condition is equivalent with another, + // so we will only check if they are literally the same condition expression + for ((inverted1, cond1), + (inverted2, cond2)) in selfvec.iter().zip(othervec.iter()) { + if cond1 != cond2 { + return false; + } + if inverted1 != inverted2 { + return true; + } + } + false + }, + _ => false, + } + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -154,16 +219,10 @@ where T: DeclarationSpan { impl ObjectDecl where T: DeclarationSpan { - pub fn depending_on_context(obj: &T, - context: StatementContext, - conds: &[(bool, Expression)]) + pub fn with_conds(obj: &T, + conds: &Arc>) -> ObjectDecl { - match context { - StatementContext::HashIfTrue | - StatementContext::HashIfElse => - ObjectDecl::conditional(obj, conds), - _ => ObjectDecl::always(obj), - } + ObjectDecl::conditional(obj, conds) } pub fn always(obj: &T) -> ObjectDecl { @@ -174,10 +233,10 @@ where T: DeclarationSpan { } } pub fn conditional(obj: &T, - conds: &[(bool, Expression)]) + conds: &Arc>) -> ObjectDecl { ObjectDecl { - cond: ExistCondition::Conditional(conds.to_vec()), + cond: ExistCondition::Conditional(Arc::clone(conds)), obj: obj.clone(), spec: StatementSpec::empty(), } @@ -330,7 +389,7 @@ impl StatementSpec { // Flattens hashifs fn flatten_hashif_branch(context: StatementContext, stmnts: &Statements, - conds: Vec<(bool, Expression)>, + conds: Arc>, report: &mut Vec) -> StatementSpec { let mut objects = vec![]; @@ -347,24 +406,23 @@ fn flatten_hashif_branch(context: StatementContext, let mut errors = vec![]; let mut templates = vec![]; for inst in &stmnts.instantiations { - instantiations.push(ObjectDecl::depending_on_context( - inst, context, &conds)); + instantiations.push(ObjectDecl::with_conds( + inst, &conds)); } for err in &stmnts.errors { - errors.push(ObjectDecl::depending_on_context( - err, context, &conds)); + errors.push(ObjectDecl::with_conds( + err, &conds)); } for ineach in stmnts.ineachs.iter() { let spec = flatten_hashif_branch(StatementContext::InEach, &ineach.statements, - vec![], report); + Arc::default(), report); ineachs.push( ObjectDecl { cond: match context { StatementContext::HashIfTrue | StatementContext::HashIfElse => - ExistCondition::Conditional( - conds.clone()), + ExistCondition::Conditional(Arc::clone(&conds)), _ => ExistCondition::Always, }, obj: ineach.clone(), @@ -412,7 +470,7 @@ fn flatten_hashif_branch(context: StatementContext, let subspec = flatten_hashif_branch( StatementContext::Template, &tmpl.statements, - vec![], + Arc::default(), report); templates.push( ObjectDecl { @@ -444,8 +502,8 @@ fn flatten_hashif_branch(context: StatementContext, // TODO: Verify that conditions are constants // or builtin parameters DMLObject::Import(import) => - imports.push(ObjectDecl::depending_on_context( - import, context, &conds)), + imports.push(ObjectDecl::with_conds( + import, &conds)), DMLObject::Loggroup(loggroup) => report.push(LocalDMLError { range: loggroup.span.range, @@ -464,14 +522,13 @@ fn flatten_hashif_branch(context: StatementContext, let subspec = flatten_hashif_branch( StatementContext::Object, &compobj.statements, - vec![], report); + Arc::clone(&conds), report); objects.push( ObjectDecl { cond: match context { StatementContext::HashIfTrue | StatementContext::HashIfElse => - ExistCondition::Conditional( - conds.clone()), + ExistCondition::Conditional(Arc::clone(&conds)), _ => ExistCondition::Always, }, obj: compobj.clone(), @@ -479,11 +536,11 @@ fn flatten_hashif_branch(context: StatementContext, }); }, DMLObject::Export(exp) => - exports.push(ObjectDecl::depending_on_context( - exp, context, &conds)), + exports.push(ObjectDecl::with_conds( + exp, &conds)), DMLObject::Hook(hook)=> - hooks.push(ObjectDecl::depending_on_context( - hook, context, &conds)), + hooks.push(ObjectDecl::with_conds( + hook, &conds)), DMLObject::Method(meth) => // This error has already been reported, but // we also need to clear our and pretend @@ -496,40 +553,40 @@ fn flatten_hashif_branch(context: StatementContext, modified_method.modifier = MethodModifier::None; methods.push( - ObjectDecl::depending_on_context( - &modified_method, context, &conds)) + ObjectDecl::with_conds( + &modified_method, &conds)) } else { methods.push( - ObjectDecl::depending_on_context( - meth, context, &conds)) + ObjectDecl::with_conds( + meth, &conds)) }, DMLObject::Saved(saved) => - saveds.push(ObjectDecl::depending_on_context( - saved, context, &conds)), + saveds.push(ObjectDecl::with_conds( + saved, &conds)), DMLObject::Session(sess) => - sessions.push(ObjectDecl::depending_on_context( - sess, context, &conds)), + sessions.push(ObjectDecl::with_conds( + sess, &conds)), DMLObject::Parameter(param) => - params.push(ObjectDecl::depending_on_context( - param, context, &conds)), + params.push(ObjectDecl::with_conds( + param, &conds)), } }, DMLStatement::HashIf(hi) => { - let mut true_conds = conds.clone(); + let mut true_conds = (*conds).clone(); true_conds.push( (true, hi.condition.clone())); - let mut false_conds = conds.clone(); + let mut false_conds = (*conds).clone(); false_conds.push( (false, hi.condition.clone())); let truebranchspec = flatten_hashif_branch( StatementContext::HashIfTrue, &hi.truebranch, - true_conds, + Arc::new(true_conds), report); let falsebranchspec = flatten_hashif_branch( StatementContext::HashIfElse, &hi.falsebranch, - false_conds, + Arc::new(false_conds), report); truebranchspec.consume(&mut objects, &mut sessions, @@ -695,7 +752,7 @@ impl TopLevel { for ineach in &statements.ineachs { let spec = flatten_hashif_branch(StatementContext::Object, &ineach.statements, - vec![], report); + Arc::default(), report); ineachs.push( ObjectDecl { cond: ExistCondition::Always, @@ -748,7 +805,7 @@ impl TopLevel { let subspec = flatten_hashif_branch( StatementContext::Template, &tmpl.statements, - vec![], + Arc::default(), report); templates.push( ObjectDecl { @@ -768,7 +825,7 @@ impl TopLevel { let subspec = flatten_hashif_branch( StatementContext::Object, &compobj.statements, - vec![], report); + Arc::default(), report); objects.push( ObjectDecl { cond: ExistCondition::Always, @@ -803,8 +860,8 @@ impl TopLevel { DMLStatement::HashIf(hi) => { // First element of the cond tuple is whether the // condition is NOT in an elsebranch - let true_conds = vec![(true, hi.condition.clone())]; - let false_conds = vec![(false, hi.condition.clone())]; + let true_conds = Arc::new(vec![(true, hi.condition.clone())]); + let false_conds = Arc::new(vec![(false, hi.condition.clone())]); let truebranchspec = flatten_hashif_branch( StatementContext::HashIfTrue, &hi.truebranch, diff --git a/src/analysis/templating/evaluation.rs b/src/analysis/templating/evaluation.rs new file mode 100644 index 0000000..60bf47d --- /dev/null +++ b/src/analysis/templating/evaluation.rs @@ -0,0 +1,265 @@ +// © 2026 Intel Corporation +// SPDX-License-Identifier: Apache-2.0 and MIT + +// Defines traits and functions for evaluating expressions within an analysis + +use lsp_types::DiagnosticSeverity; + +use crate::analysis::{DMLError, DeclarationSpan, templating::types::DMLResolvedType}; + +/// Structure that will contain the information available at the point of +/// evaluation, can be extended with additional optional fields in the future +#[derive(Debug, Clone)] +pub struct EvaluationContext { +} + +#[allow(clippy::new_without_default)] +impl EvaluationContext { + pub fn new() -> Self { + EvaluationContext { + } + } +} + +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum EvaluatedValue { + // We'll use this for storing both signed, unsigned, and endian values + // the disambiguation between them is done by the type in the + // EvaluationResult struct + Int(i64), + Bool(bool), + DMLObject(()), // TODO: How to specify this? Path? Arc? + // TODO: Other, more obtuse, types (floats, pointers, strings?) +} + +impl EvaluatedValue { + pub fn as_int(&self) -> Option { + match self { + EvaluatedValue::Int(i) => Some(*i), + _ => None, + } + } + + pub fn as_bool(&self) -> Option { + match self { + EvaluatedValue::Bool(b) => Some(*b), + _ => None, + } + } +} + +#[derive(Debug, Clone)] +pub struct EvaluationResult { + pub value: Option, + // Wether this value is 'true constant' and can be used to resolve #if's at compile time + pub constant: Option, + pub typed: Option, +} + +impl EvaluationResult { + pub fn empty() -> Self { + EvaluationResult { + value: None, + constant: None, + typed: None, + } + } + pub fn as_int(&self) -> Option { + self.value.as_ref().and_then(EvaluatedValue::as_int) + } + + pub fn as_bool(&self) -> Option { + self.value.as_ref().and_then(EvaluatedValue::as_bool) + } +} + +pub trait Evaluatable { + fn evaluate(&self, context: &EvaluationContext, report: &mut Vec) -> EvaluationResult; +} + +// TODO: As we make improvements, we can add Evaluatable trait impls +// for various types here, for now we allow a small sub-set only + +// Note: we fully qualify the types here, as we may want to implement this trait for similarly-named +// types from different stages in the analysis +impl Evaluatable for crate::analysis::structure::expressions::Expression { + fn evaluate(&self, context: &EvaluationContext, report: &mut Vec) -> EvaluationResult { + match self.as_ref() { + crate::analysis::structure::expressions::ExpressionKind::TertiaryExpression(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::BinaryExpression(binexpr) => binexpr.evaluate(context, report), + crate::analysis::structure::expressions::ExpressionKind::UnaryExpression(uexpr) => uexpr.evaluate(context, report), + crate::analysis::structure::expressions::ExpressionKind::IndexExpression(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::Slice(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::Identifier(ident) => ident.evaluate(context, report), + crate::analysis::structure::expressions::ExpressionKind::IntegerLiteral(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::StringLiteral(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::CharLiteral(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::MemberLiteral(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::FloatLiteral(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::FunctionCall(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::CastExpression(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::NewExpression(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::SizeOf(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::SizeOfType(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::ConstantList(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::EachIn(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::AutoObjectRef(_, _) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::Undefined(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::ExpressionKind::Unknown(_) => EvaluationResult::empty(), + } + } +} + +fn not_operation(inner: EvaluationResult) -> EvaluationResult { + // For now, we only support the not operator for boolean values, but ! is applicable to + // other types as well + // TODO: We don't have a type system yet, but when we do that should guide the semantics of this + match inner.value { + Some(EvaluatedValue::Bool(b)) => EvaluationResult { + value: Some(EvaluatedValue::Bool(!b)), + constant: inner.constant, + typed: inner.typed, + }, + _ => EvaluationResult::empty(), + } +} + +impl Evaluatable for crate::analysis::structure::expressions::UnaryExpression { + fn evaluate(&self, context: &EvaluationContext, report: &mut Vec) -> EvaluationResult { + let inner_evaluated = self.operand.evaluate(context, report); + // Break out operations to subfunctions so as to not clutter this function too much + match self.operator { + crate::analysis::structure::expressions::UnaryOp::Not => not_operation(inner_evaluated), + crate::analysis::structure::expressions::UnaryOp::PlusPlus => EvaluationResult::empty(), + crate::analysis::structure::expressions::UnaryOp::MinusMinus => EvaluationResult::empty(), + crate::analysis::structure::expressions::UnaryOp::Defined => EvaluationResult::empty(), + crate::analysis::structure::expressions::UnaryOp::Minus => EvaluationResult::empty(), + crate::analysis::structure::expressions::UnaryOp::BinNot => EvaluationResult::empty(), + crate::analysis::structure::expressions::UnaryOp::AddressOf => EvaluationResult::empty(), + crate::analysis::structure::expressions::UnaryOp::Dereference => EvaluationResult::empty(), + } + } +} + +impl Evaluatable for crate::analysis::structure::expressions::Identifier { + fn evaluate(&self, _context: &EvaluationContext, _report: &mut Vec) -> EvaluationResult { + // TODO: eventually we will do symbol lookups based on the evaluation context, + // for now we hard-code some constants which are treated especially by DMLC + match self.name.val.as_str() { + "true" => EvaluationResult { + value: Some(EvaluatedValue::Bool(true)), + constant: Some(true), + typed: None, + }, + "false" => EvaluationResult { + value: Some(EvaluatedValue::Bool(false)), + constant: Some(true), + typed: None, + }, + "dml_1_2" => EvaluationResult { + value: Some(EvaluatedValue::Bool(false)), + constant: Some(true), + typed: None, + }, + "dml_1_4" => EvaluationResult { + value: Some(EvaluatedValue::Bool(true)), + constant: Some(true), + typed: None, + }, + _ => EvaluationResult::empty(), + } + } +} + +impl Evaluatable for crate::analysis::structure::expressions::BinaryExpression { + fn evaluate(&self, context: &EvaluationContext, report: &mut Vec) -> EvaluationResult { + let left_evaluated = self.left.evaluate(context, report); + let right_evaluated = self.right.evaluate(context, report); + match &self.operator { + crate::analysis::structure::expressions::BinOp::Math(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::BinOp::Comp(_) => EvaluationResult::empty(), + crate::analysis::structure::expressions::BinOp::Logic(logic_op) => logic_expression( + left_evaluated, right_evaluated, logic_op, report, + ), + } + } +} + +// This is annoying enough to break out into its own function +fn combine_constants(constant_info: &[Option]) -> Option { + for constant in constant_info { + match constant { + Some(true) => (), + Some(false) => return Some(false), + None => return None, + } + } + Some(true) +} + +// These are meaningfull to evaluate already, since we have sub-expressions used for logic +fn logic_expression(left: EvaluationResult, + right: EvaluationResult, + logic_op: &crate::analysis::structure::expressions::LogicOp, + _report: &mut Vec) -> EvaluationResult { + // TODO: we can verify types before matching on operator + // NOTE: Short-cutting for indeterminate expressions has some arbitrary heuristic decisions made, in essence we assume + // that the indeterminate value does not exist. So A | B -> A if B is indeterminate, and A & B similarly becomes A + let (constant, logic_result) = match logic_op { + crate::analysis::structure::expressions::LogicOp::And => { + match (left.as_bool(), right.as_bool()) { + // TODO: Verify that DMLC short-circuiting actually short-circuits the constant property like this + (Some(b), None) => (left.constant, Some(b)), + (None, b) => (right.constant, b), + (Some(b1), Some(b2)) => (combine_constants(&[left.constant, right.constant]), Some(b1 && b2)), + + } + }, + crate::analysis::structure::expressions::LogicOp::Or => { + match (left.as_bool(), right.as_bool()) { + (Some(false), Some(false)) => (combine_constants(&[left.constant, right.constant]), Some(false)), + (b, None) => (left.constant, b), + (_, b) => (right.constant, b), + } + }, + }; + EvaluationResult { + value: logic_result.map(EvaluatedValue::Bool), + constant, + typed: None, + } +} + +impl Evaluatable for crate::analysis::structure::expressions::TertiaryExpression { + fn evaluate(&self, context: &EvaluationContext, report: &mut Vec) -> EvaluationResult { + let left_evaluated = self.left.evaluate(context, report); + // Patch in the constant-ness of the condition, as that will modify the constant-ness of the result + let mut middle_evaluated = self.middle.evaluate(context, report); + let mut right_evaluated = self.right.evaluate(context, report); + middle_evaluated.constant = combine_constants(&[left_evaluated.constant, middle_evaluated.constant]); + right_evaluated.constant = combine_constants(&[left_evaluated.constant, right_evaluated.constant]); + match &self.operator { + crate::analysis::structure::expressions::TertiaryOp::Cond => + if left_evaluated.as_bool() == Some(true) { + middle_evaluated + } else { + right_evaluated + }, + crate::analysis::structure::expressions::TertiaryOp::HashCond => { + if left_evaluated.constant == Some(false) { + report.push(DMLError { + span: *self.left.span(), + severity: Some(DiagnosticSeverity::ERROR), + description: "Condition in #if must be constant".to_string(), + related: vec![], + }); + } + if left_evaluated.as_bool() == Some(true) { + middle_evaluated + } else { + right_evaluated + } + }, + } + } +} diff --git a/src/analysis/templating/mod.rs b/src/analysis/templating/mod.rs index d9a7e15..3592823 100644 --- a/src/analysis/templating/mod.rs +++ b/src/analysis/templating/mod.rs @@ -5,6 +5,7 @@ pub mod objects; pub mod topology; pub mod types; pub mod traits; +pub mod evaluation; use crate::analysis::templating::types::DMLResolvedType; use crate::analysis::structure::expressions::DMLString; diff --git a/src/analysis/templating/objects.rs b/src/analysis/templating/objects.rs index 639d400..30aec74 100644 --- a/src/analysis/templating/objects.rs +++ b/src/analysis/templating/objects.rs @@ -4,6 +4,7 @@ use std::collections::{HashMap, HashSet}; use std::iter; use std::sync::Arc; +use crate::analysis::templating::evaluation::EvaluationContext; use crate::logging::{debug, trace, error}; use lsp_types::DiagnosticSeverity; use slotmap::{DefaultKey, Key, SlotMap}; @@ -770,16 +771,20 @@ impl DMLAmbiguousDef { } } - pub fn get_likely_definition(&self) -> &T { + pub fn get_likely_definition_info(&self) -> &(ExistCondition, T) { if let Some(def) = self.used_definitions.first() { - &def.1 + def } else if let Some(def) = self.definitions.first() { - &def.1 + def } else { - &self.declarations.first().unwrap().1 + self.declarations.first().unwrap() } } + pub fn get_likely_definition(&self) -> &T { + &self.get_likely_definition_info().1 + } + pub fn get_last_declaration(&self) -> &T { if let Some(decl) = self.declarations.last() { &decl.1 @@ -807,6 +812,13 @@ impl DMLAmbiguousDef { pub fn is_unambiguous(&self) -> bool { self.declarations.len() == 1 && self.used_definitions.len() == 1 } + pub fn get_all_definitions(&self) -> Vec<(&ExistCondition, &T)> { + self.used_definitions.iter() + .chain(self.definitions.iter()) + .chain(self.declarations.iter()) + .map(|(cond, def)| (cond, def)) + .collect() + } } impl DMLNamed for DMLAmbiguousDef @@ -958,7 +970,8 @@ impl DMLCompositeObject { // Returns all spans of final actually used ineachspecs, needed for // implementations tracking for composite objects fn add_template_specs(obj_specs: &mut Vec>, - source_each_stmts: &InEachSpec) -> Vec{ + source_each_stmts: &InEachSpec, + report: &mut Vec) -> Vec{ let mut each_stmts = source_each_stmts.clone(); // TODO: We need to handle conditional imports and is-es here, as these are // allowed in _some_ cases. For now, we merely pretend the conditions do not @@ -987,9 +1000,11 @@ fn add_template_specs(obj_specs: &mut Vec>, let mut used_ineach_spans = vec![]; - while let Some((_, tpl)) = queue.pop() { - // TODO: here we would have to consider cond when conditional - // is/imports exist + while let Some((cond, tpl)) = queue.pop() { + if !cond.exists(&EvaluationContext::new(), report) { + continue; + } + if used_templates.contains(&tpl.name) { continue; } @@ -1086,9 +1101,9 @@ fn add_hooks(obj: &mut DMLCompositeObject, } fn add_constants(obj: &mut DMLCompositeObject, - constants: Vec) { + constants: Vec>) { for constant in constants { - obj.add_shallow_component(DMLShallowObjectVariant::Constant(constant)); + obj.add_shallow_component(DMLShallowObjectVariant::Constant(constant.obj)); } } @@ -1099,7 +1114,7 @@ enum VariableKind { fn add_sessions(obj: &mut DMLCompositeObject, sessions: SessionMapping) { - for (_, (used, mut decls)) in sessions { + for (_, (used, _, mut decls)) in sessions { if used { let (_, (s, i)) = decls.swap_remove(0); handle_variable(obj, s, i, VariableKind::Session) @@ -1109,7 +1124,7 @@ fn add_sessions(obj: &mut DMLCompositeObject, fn add_saveds(obj: &mut DMLCompositeObject, saveds: SavedMapping) { - for (_, (used, mut decls)) in saveds { + for (_, (used, _, mut decls)) in saveds { if used { let (_, (s, i)) = decls.swap_remove(0); handle_variable(obj, s, i, VariableKind::Saved) @@ -1179,6 +1194,9 @@ fn gather_parameters<'t>(obj_loc: &ZeroSpan, // Add code-decl parameters for spec in specs { + if !spec.condition.exists(&EvaluationContext::new(), report) { + continue; + } for param in &spec.params { let new_decl = (param.clone(), spec.rank.clone()); if let Some(e) = parameters.get_mut( @@ -1312,24 +1330,33 @@ fn resolve_parameter(obj_loc: &ZeroSpan, .collect(); trace!("Top defs for {} are {:?}", name, overriding_defs); if overriding_defs.len() > 1 { - debug!("Conflicting assignment for {:?}, {:?}", - name, overriding_defs); // in dmlc, there is a preference for blaming default declarations. // we will merely blame all of them, starting at the preferred def // TODO: This can be improved for specific cases where a name // collision error is more appropriate - let mut rest = overriding_defs.iter(); - let (first, _) = rest.next().unwrap(); - report.push(DMLError { - span: *first.obj.span(), - description: format!( - "Conflicting assignments to parameter '{}'", name), - related: rest.map( - |(def2, _)| (*def2.obj.span(), + let mut rest: Vec<_> = overriding_defs.clone(); + + // Sort declarations by span for consistent reporting + rest.sort_by_key(|o|o.0.loc_span()); + + // TODO: This could be improved to report conflicts between rest + let (first, rest) = rest.split_first().unwrap(); + let conflicts: Vec<_> = rest.iter().filter( + |(o, _)|!first.0.cond.guaranteed_excluded_from(&o.cond) + ).collect(); + + if !conflicts.is_empty() { + report.push(DMLError { + span: *first.0.obj.span(), + description: format!( + "Conflicting assignments to parameter '{}'", name), + related: conflicts.into_iter().map( + |(def2, _)| (*def2.obj.span(), "Conflicting assignment here" .to_string())).collect(), - severity: Some(DiagnosticSeverity::ERROR), - }); + severity: Some(DiagnosticSeverity::ERROR), + }); + } } let bad_overrides: Vec<&(ObjectDecl, Rank)> @@ -1421,7 +1448,7 @@ fn handle_variable(obj: &mut DMLCompositeObject, // Maps name to (used, definitions), where definitions is a vector of // (Rank, Method) tuples -type MethodMapping = HashMap)>; +type MethodMapping = HashMap)>; // Maps name to (used, definitions), where definitions is a vector of // (Rank, Decl, Spec) tuples type ObjectMapping = HashMap)>)>; // Maps name to (used, definitions), similar to methodmapping type SavedMapping = HashMap))>)>; type SessionMapping = HashMap))>)>; type HookMapping = HashMap)>)>; @@ -1441,8 +1468,8 @@ type HookMapping = HashMap)>)>; // Figure out symbol mappings for these specs // reports unguarded error // the hashmap is the symbol mapping -type CollectedSymbols = (HashMap)>, - Vec, +type CollectedSymbols = (HashMap)>, + Vec>, SavedMapping, SessionMapping, MethodMapping, HookMapping, ObjectMapping); fn collect_symbols(parameters: &[DMLParameter], @@ -1461,22 +1488,30 @@ fn collect_symbols(parameters: &[DMLParameter], let mut methods = MethodMapping::default(); let mut subobjs = ObjectMapping::default(); let mut hooks = HookMapping::default(); - let mut constants: Vec = vec![]; + let mut constants: Vec> = vec![]; + + // In order to not overly complicate the types, for spec in obj_specs { + // Check if the spec is valid at all + // NOTE: we do need to also check the conditions of all sub-statements, as they + // may have their own conditionals + if !spec.condition.exists(&EvaluationContext::new(), report) { + continue; + } for error in &spec.errors { - // TODO: evaluate conditions - // for now, conservatively only report the errors for - // things not behind hashifs - if error.cond == ExistCondition::Always { - report.push(DMLError { - span: *error.span(), - // TODO: early-evaluate the error message, somehow - description: "unguarded error statement".to_string(), - related: vec![], - severity: Some(DiagnosticSeverity::ERROR), - }); + // For errors in particular, we will be more lenient and only + // report them when they are guaranteed to exist + if !error.cond.guaranteed_exists(&EvaluationContext::new(), report) { + continue; } + report.push(DMLError { + span: *error.span(), + // TODO: early-evaluate the error message, somehow + description: "unguarded error statement".to_string(), + related: vec![], + severity: Some(DiagnosticSeverity::ERROR), + }); } // TODO: How to handle extra initializers here? // Its a bit to early to eagerly evaluate them, but discarding @@ -1484,9 +1519,15 @@ fn collect_symbols(parameters: &[DMLParameter], // (extra vars is not a problem, since they just become un-inited // declarations) for constant in &spec.constants { - constants.push(constant.obj.clone()); + if !constant.cond.exists(&EvaluationContext::new(), report) { + continue; + } + constants.push(constant.clone()); } for saved_objectdecl in &spec.saveds { + if !saved_objectdecl.cond.exists(&EvaluationContext::new(), report) { + continue; + } // Pad initializers with 'None' values, so that we handle all the // variables let saved = &saved_objectdecl.obj; @@ -1496,14 +1537,18 @@ fn collect_symbols(parameters: &[DMLParameter], let to_insert = (spec.rank.clone(), (var.clone(), init.cloned())); let name = var.object.name.val.clone(); - if let Some((_, e)) = saveds.get_mut(&name) { + if let Some((_, _, e)) = saveds.get_mut(&name) { e.push(to_insert); } else { - saveds.insert(name, (false, vec![to_insert])); + saveds.insert(name, (false, saved_objectdecl.cond.clone(), + vec![to_insert])); } } } for session_objectdecl in &spec.sessions { + if !session_objectdecl.cond.exists(&EvaluationContext::new(), report) { + continue; + } let session = &session_objectdecl.obj; for (var, init) in session.vars.iter().zip( session.values.iter().map(|e|Some(e)) @@ -1511,15 +1556,20 @@ fn collect_symbols(parameters: &[DMLParameter], let to_insert = (spec.rank.clone(), (var.clone(), init.cloned())); let name = var.object.name.val.clone(); - if let Some((_,e)) = sessions.get_mut(&name) { + if let Some((_, _, e)) = sessions.get_mut(&name) { e.push(to_insert); } else { - sessions.insert(name, (false, vec![to_insert])); + sessions.insert(name, (false, session_objectdecl.cond.clone(), + vec![to_insert])); } } } for method in &spec.methods { + if !method.cond.exists(&EvaluationContext::new(), report) { + continue; + } let to_insert = (spec.rank.clone(), + method.cond.clone(), MethodDecl::from_content(&method.obj, report)); let name = method.obj.object.name.val.clone(); if let Some((_, e)) = methods.get_mut(&name) { @@ -1529,6 +1579,9 @@ fn collect_symbols(parameters: &[DMLParameter], } } for hook in &spec.hooks { + if !hook.cond.exists(&EvaluationContext::new(), report) { + continue; + } let to_insert = (spec.rank.clone(), hook.clone()); let name = &hook.obj.name().val; if let Some((_, e)) = hooks.get_mut(name) { @@ -1539,6 +1592,9 @@ fn collect_symbols(parameters: &[DMLParameter], } for (subobj, spec) in &spec.subobjs { + if !subobj.cond.exists(&EvaluationContext::new(), report) { + continue; + } let to_insert = (spec.rank.clone(), subobj.clone(), Arc::clone(spec)); let name = subobj.obj.object.name.val.clone(); @@ -1552,14 +1608,14 @@ fn collect_symbols(parameters: &[DMLParameter], // TODO: the sorting is insertion sort anyway, we could // be sorting this while inserting - for (_, decls) in saveds.values_mut() { + for (_, _, decls) in saveds.values_mut() { partial_sort_by_key_in_place(decls, |(r, _)|r); } - for (_, decls) in sessions.values_mut() { + for (_, _, decls) in sessions.values_mut() { partial_sort_by_key_in_place(decls, |(r, _)|r); } for (_, decls) in methods.values_mut() { - partial_sort_by_key_in_place(decls, |(r, _)|r); + partial_sort_by_key_in_place(decls, |(r, _, _)|r); } for (_, decls) in hooks.values_mut() { partial_sort_by_key_in_place(decls, |(r, _)|r); @@ -1568,78 +1624,102 @@ fn collect_symbols(parameters: &[DMLParameter], partial_sort_by_key_in_place(decls, |(r, _, _)|r); } - // Map names to most relevant declaration and colliding declarations + // Map names to most relevant declaration, its exist condition, and colliding declarations // This creates an order for symbol definition precedence // constant > parameter > subobj > method > saved > session // Grab the most-relevant decl from each ambiguousdecl, if they have // parameter-to-parameter collisions then that has already been reported - let mut symbols: HashMap)> + let mut symbols: HashMap)> = HashMap::new(); + // Helper function, stores spans to report for name conflicts if the + // conditions are not provably exclusive + fn store_if_not_excluded(cond1: &ExistCondition, cond2: &ExistCondition, + span: &ZeroSpan, + store_in: &mut Vec) { + if !cond1.guaranteed_excluded_from(cond2) { + store_in.push(*span); + } + } + // NOTE: constants are top-level only, so ordering doesn't really matter for constant in &constants { - if let Some((_, rest)) = symbols.get_mut(constant.name().val.as_str()) { - rest.push(*constant.loc_span()); + if let Some((cond, _, rest)) = symbols.get_mut(constant.obj.name().val.as_str()) { + store_if_not_excluded(cond, &constant.cond, constant.loc_span(), rest); } else { - symbols.insert(constant.name().val.clone(), - (*constant.loc_span(), vec![])); + symbols.insert(constant.obj.name().val.clone(), + (constant.cond.clone(), *constant.loc_span(), vec![])); } } for parameter in parameters { - let maybe_auth_decl_span = *parameter.get_likely_definition() - .object.loc_span(); - let name = parameter.get_likely_definition().object.name.val.clone(); - if let Some((_, rest)) = symbols.get_mut(&name) { - rest.push(maybe_auth_decl_span); + let (cond, likely_def) = parameter.get_likely_definition_info(); + let maybe_auth_decl_span = *likely_def.object.loc_span(); + let name = likely_def.object.name.val.clone(); + if let Some((auth_cond, _, rest)) = symbols.get_mut(&name) { + // For this and other cases where we have multiple possible conflicts, we check + // the declaration of each one and report all the ones that are not provably + // exclusive with the auth cond + for (sub_cond, param) in parameter.get_all_definitions() { + store_if_not_excluded(sub_cond, auth_cond, param.span(), rest); + } } else { - symbols.insert(name, (maybe_auth_decl_span, vec![])); + symbols.insert(name, (cond.clone(), maybe_auth_decl_span, vec![])); } } for (name, (used, objs)) in &mut subobjs { - let maybe_auth_decl_span = objs.first().unwrap() - .1.obj.object.name.span; - if let Some((_, rest)) = symbols.get_mut(name) { - rest.push(maybe_auth_decl_span); + let first_obj = objs.first().unwrap(); + let maybe_auth_decl_span = first_obj.1.obj.object.name.span; + if let Some((cond, _, rest)) = symbols.get_mut(name) { + for (_, subobj, _) in objs { + store_if_not_excluded(cond, &subobj.cond, subobj.span(), rest); + } } else { *used = true; - symbols.insert(name.to_string(), (maybe_auth_decl_span, vec![])); + symbols.insert(name.to_string(), (first_obj.1.cond.clone(), maybe_auth_decl_span, vec![])); } } for (name, (used, methods)) in &mut methods { - let maybe_auth_decl_span = methods.first().unwrap() - .1.name.span; - if let Some((_, rest)) = symbols.get_mut(name) { - rest.push(maybe_auth_decl_span); + let (_, cond, method) = methods.first().unwrap(); + let maybe_auth_decl_span = method.name.span; + if let Some((cond, _, rest)) = symbols.get_mut(name) { + for (_, sub_cond, sub_method) in methods { + store_if_not_excluded(sub_cond, cond, sub_method.span(), rest); + } } else { *used = true; - symbols.insert(name.to_string(), (maybe_auth_decl_span, vec![])); + symbols.insert(name.to_string(), (cond.clone(), maybe_auth_decl_span, vec![])); } } - for (name, (used, decls)) in &mut saveds { + // Each entry in saveds here is one particular cond, but contains several names. + // So check for name collision first, and then use the cond to mark all such names + // conflicting + for (name, (used, cond1, decls)) in &mut saveds { for (_, (var, _)) in decls { let maybe_auth_decl_span = var.object.name.span; - if let Some((_, rest)) = symbols.get_mut(name) { - rest.push(maybe_auth_decl_span); + if let Some((cond2, _, rest)) = symbols.get_mut(name) { + store_if_not_excluded(cond1, cond2, &maybe_auth_decl_span, rest); } else { *used = true; + // We will duplicate the variable cond here for each declaration name, + // which is inefficient but in pratice should be small symbols.insert(name.to_string(), - (maybe_auth_decl_span, vec![])); + (cond1.clone(), maybe_auth_decl_span, vec![])); } } } - for (name, (used, decls)) in &mut sessions { + for (name, (used, cond1, decls)) in &mut sessions { for (_, (var, _)) in decls { let maybe_auth_decl_span = var.object.name.span; - if let Some((_, rest)) = symbols.get_mut(name) { - rest.push(maybe_auth_decl_span); + if let Some((cond2, _, rest)) = symbols.get_mut(name) { + store_if_not_excluded(cond1, cond2, &maybe_auth_decl_span, rest); } else { *used = true; symbols.insert(name.to_string(), - (maybe_auth_decl_span, vec![])); + (cond1.clone(), maybe_auth_decl_span, vec![])); } } } @@ -1647,17 +1727,17 @@ fn collect_symbols(parameters: &[DMLParameter], for (name, (used, decls)) in &mut hooks { for (_, hook) in decls { let maybe_auth_decl_span = hook.obj.name().span; - if let Some((_, rest)) = symbols.get_mut(name) { - rest.push(maybe_auth_decl_span); + if let Some((cond, _, rest)) = symbols.get_mut(name) { + store_if_not_excluded(&hook.cond, cond, &maybe_auth_decl_span, rest); } else { *used = true; symbols.insert(name.to_string(), - (maybe_auth_decl_span, vec![])); + (hook.cond.clone(), maybe_auth_decl_span, vec![])); } } } - for (name, (auth, others)) in &symbols { + for (name, (_, auth, others)) in &symbols { if !others.is_empty() { report.push(DMLError { span: (*auth), @@ -1709,7 +1789,14 @@ fn merge_composite_subobj<'c>(name: String, let mut array_info: Vec<(ArrayDim, Vec<&ZeroSpan>)> = auth_obj.obj.dims.iter() .map(|d|(d.clone(), vec![])).collect(); + // TODO: Starting to seem likely that we might double-report from + // bad/invalid conds. Might want to formalize expression resolution in such + // a way that we convert constant expressions once first, and then + // convert remaining expressions later for (decl, _) in &specs { + if !decl.cond.exists(&EvaluationContext::new(), report) { + continue; + } if decl.obj.dims.len() != array_info.len() { // When an object with no array decl conflicts with an object // with one, blame the object decl @@ -1924,7 +2011,7 @@ fn add_methods(obj: &mut DMLCompositeObject, trace!("Handling method {}, which is declared by {:?}", name, regular_decls); let all_decls: Vec<(&Rank, &MethodDecl)> - = regular_decls.iter().map(|(a, b)|(a, b)).collect(); + = regular_decls.iter().map(|(a, _, b)|(a, b)).collect(); let (default_map, method_order, decl_map) = sort_method_decls(&all_decls); let mut decl_to_method: HashMap> @@ -2273,7 +2360,7 @@ pub fn make_object(loc: ZeroSpan, let direct_decls: Vec> = obj_specs.clone(); let mut each_stmts = parent_each_stmts.clone(); - let used_ineach_locs = add_template_specs(&mut obj_specs, &each_stmts); + let used_ineach_locs = add_template_specs(&mut obj_specs, &each_stmts, report); add_template_ineachs(&obj_specs, &mut each_stmts); trace!("Has specs at {:?}", obj_specs.iter().map(|rc|rc.loc)