Better handling around #if conditionals - #227
Conversation
JonatanWaern
commented
Jun 9, 2026
- Refactor how we store object conds for objectdecls
- Add logic to select between hashif branches
88ae3a0 to
0c8cd42
Compare
Signed-off-by: Jonatan Waern <jonatan.waern@intel.com>
Rather speculative for now, as exact future info required to make this choice isn't well-known Signed-off-by: Jonatan Waern <jonatan.waern@intel.com> Summary: hoooolder info: patch template saved to `-`
0c8cd42 to
b7a331b
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
This PR improves handling of #if/#else conditionals during analysis by introducing expression evaluation support and using existence conditions to avoid reporting conflicts across mutually exclusive branches.
Changes:
- Added an
evaluationmodule to evaluate a small subset of expressions for conditional existence checks. - Refactored
ExistConditionstorage to useArcand added helpers (exists,guaranteed_exists,guaranteed_excluded_from) to reason about conditional branches. - Updated object/spec symbol collection and conflict detection to consider evaluated
#ifconditions and avoid conflicts between#ifand corresponding#else.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/analysis/templating/objects.rs | Uses ExistCondition evaluation to filter specs/decls and refines conflict reporting to ignore provably exclusive branches |
| src/analysis/templating/mod.rs | Exposes the new evaluation module |
| src/analysis/templating/evaluation.rs | Introduces expression evaluation utilities used for #if condition resolution |
| src/analysis/structure/toplevel.rs | Stores conditional stacks in Arc and adds existence/exclusion helpers on ExistCondition |
| CHANGELOG.md | Documents improved conflict handling across #if/#else and built-in version-condition behavior |
Suppressed comments (1)
src/analysis/structure/toplevel.rs:97
is_samecan returntruefor different-length conditional stacks becausezip()truncates to the shorter iterator (e.g.,[A]vs[A,B]will returntrue). Add an explicit length equality check before the loop (or compare the full vectors) so only identical conditional stacks are treated as the same.
(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 ((_, cond1),
(_, cond2)) in selfvec.iter().zip(othervec.iter()) {
if cond1 != cond2 {
return false;
}
}
true
},
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| type SavedMapping = HashMap<String, | ||
| (bool, | ||
| (bool, ExistCondition, | ||
| Vec<(Rank, | ||
| (VariableDecl, Option<Initializer>))>)>; | ||
| type SessionMapping = HashMap<String, | ||
| (bool, | ||
| (bool, ExistCondition, | ||
| Vec<(Rank, | ||
| (VariableDecl, Option<Initializer>))>)>; |
| saveds.insert(name, (false, saved_objectdecl.cond.clone(), | ||
| vec![to_insert])); |
| sessions.insert(name, (false, session_objectdecl.cond.clone(), | ||
| vec![to_insert])); |
| ObjectDecl::conditional(obj, conds), | ||
| _ => ObjectDecl::always(obj), | ||
| } | ||
| ObjectDecl::conditional(obj, conds) |
| #[derive(Debug, Clone)] | ||
| pub struct EvaluationResult { | ||
| pub value: Option<EvaluatedValue>, | ||
| // Wether this value is 'true constant' and can be used to resolve #if's at compile time |
| Some(true) | ||
| } | ||
|
|
||
| // These are meaningfull to evaluate already, since we have sub-expressions used for logic |
| // 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, |
| } else { | ||
| *used = true; | ||
| // We will duplicate the variable cond here for each declaration name, | ||
| // which is inefficient but in pratice should be small |
| - Fix issue where the server would internally format URIs incorrectly in some cases | ||
| - The language server logs will now be in local time of whatever machine they are running on, rather than UTC. | ||
| - 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 |