diff --git a/crates/fbuild-build-engine/src/source_scanner.rs b/crates/fbuild-build-engine/src/source_scanner.rs index 1c72d85f1..ca60b18ea 100644 --- a/crates/fbuild-build-engine/src/source_scanner.rs +++ b/crates/fbuild-build-engine/src/source_scanner.rs @@ -349,17 +349,14 @@ impl SourceScanner { }) .collect::>>()?; - // Hoist every tab's `#include` directives into the prelude so that + // Hoist each tab's leading preprocessor lines into the prelude so that // auto-generated prototypes can reference types from library headers // (FastLED/fbuild#1275 — arduino-cli places prototypes *after* the - // include block for exactly this reason). Replace each hoisted - // `#include` line with a blank line in the body to preserve line - // numbering for diagnostics. - let sketch_includes = hoist_include_directives(&contents); - let stripped_contents: Vec = contents - .iter() - .map(|c| strip_include_directives(c)) - .collect(); + // include block for exactly this reason), without lifting an + // `#include` out of its `#if` (FastLED/fbuild#1440). Hoisted lines + // become blank lines in the body to preserve line numbering for + // diagnostics. + let (sketch_includes, stripped_contents) = hoist_leading_preprocessor(&contents); // Prototype extraction needs to see every tab's code, so it operates // on the plain concatenation (no #line noise). We feed it the @@ -787,44 +784,176 @@ fn walk_sources(dir: &Path) -> Vec { files } -/// Extract every `#include` directive across all tabs, deduplicated and in -/// first-seen order, for hoisting into the prelude (FastLED/fbuild#1275). -fn hoist_include_directives(contents: &[String]) -> Vec { - let mut seen = HashSet::new(); - let mut includes = Vec::new(); - for content in contents { - for line in content.lines() { - let trimmed = line.trim(); - if is_include_directive(trimmed) { - let normalized = trimmed.to_string(); - if seen.insert(normalized.clone()) { - includes.push(normalized); +/// One line of a tab's leading preprocessor region. +struct LeadingLine { + index: usize, + /// `#if` nesting depth the line sits at (0 = unconditional). + depth: usize, + /// Part of an `#include` directive. + is_include: bool, + /// A `\`-continuation of the directive on a previous line. + is_continuation: bool, +} + +/// The lines before a tab's first line of code -- blank lines, comments and +/// preprocessor directives (with `\` continuations) -- cut back to the last +/// point where `#if` nesting is closed, so a region never ends inside a +/// conditional block. +fn leading_preprocessor_region(source: &str) -> Vec { + let mut lines = Vec::new(); + let mut depth = 0usize; + let mut in_block_comment = false; + let mut continuation: Option<(usize, bool)> = None; + let mut boundary = 0usize; + + for (index, line) in source.lines().enumerate() { + let trimmed = line.trim(); + if let Some((cont_depth, cont_include)) = continuation { + lines.push(LeadingLine { + index, + depth: cont_depth, + is_include: cont_include, + is_continuation: true, + }); + if !trimmed.ends_with('\\') { + continuation = None; + } + } else if in_block_comment { + if let Some(end) = trimmed.find("*/") { + if !is_comment_tail(&trimmed[end + 2..]) { + break; + } + in_block_comment = false; + } + lines.push(LeadingLine { + index, + depth, + is_include: false, + is_continuation: false, + }); + } else if trimmed.is_empty() || trimmed.starts_with("//") { + lines.push(LeadingLine { + index, + depth, + is_include: false, + is_continuation: false, + }); + } else if let Some(after_open) = trimmed.strip_prefix("/*") { + match after_open.find("*/") { + Some(end) if !is_comment_tail(&after_open[end + 2..]) => break, + Some(_) => {} + None => in_block_comment = true, + } + lines.push(LeadingLine { + index, + depth, + is_include: false, + is_continuation: false, + }); + } else if let Some(directive) = trimmed.strip_prefix('#') { + let name: String = directive + .trim_start() + .chars() + .take_while(|c| c.is_ascii_alphanumeric() || *c == '_') + .collect(); + let line_depth = match name.as_str() { + "if" | "ifdef" | "ifndef" => { + depth += 1; + depth - 1 + } + "elif" | "else" => depth.saturating_sub(1), + "endif" => { + if depth == 0 { + break; + } + depth -= 1; + depth } + _ => depth, + }; + let is_include = name == "include"; + lines.push(LeadingLine { + index, + depth: line_depth, + is_include, + is_continuation: false, + }); + if trimmed.ends_with('\\') { + continuation = Some((line_depth, is_include)); } + } else { + break; + } + if depth == 0 && continuation.is_none() && !in_block_comment { + boundary = lines.len(); } } - includes + lines.truncate(boundary); + lines } -/// Replace every `#include` line with an empty line so that line numbering -/// is preserved when the hoisted directives are moved to the prelude -/// (FastLED/fbuild#1275). -fn strip_include_directives(source: &str) -> String { - source - .lines() - .map(|line| { - if is_include_directive(line.trim()) { - "" - } else { - line - } - }) - .collect::>() - .join("\n") +/// True when what follows a closing `*/` on the same line is nothing but +/// whitespace or a `//` comment. +fn is_comment_tail(rest: &str) -> bool { + let rest = rest.trim(); + rest.is_empty() || rest.starts_with("//") } -fn is_include_directive(trimmed: &str) -> bool { - trimmed.starts_with("#include") +/// Move each tab's leading preprocessor lines into the prelude so the +/// auto-generated prototypes, which sit in the prelude, can use types from +/// the sketch's headers (FastLED/fbuild#1275). Moved lines become blank lines +/// in the body so diagnostics keep their line numbers. +/// +/// Only lines *before the first line of code* move, and an `#include` never +/// leaves its conditional (FastLED/fbuild#1440): a Teensy-only +/// `#if ... #include #endif` used to be hoisted bare and compiled +/// on every board. +/// +/// - The first tab's region moves verbatim and in order -- includes, `#if` +/// blocks, `#define`s and comments. It precedes all other code anyway, so a +/// `#define` that configures a later header still precedes that header. +/// - Later tabs contribute only their unconditional `#include`s. Their other +/// directives stay put: moving a later tab's `#define` above the first +/// tab's code would change what that code sees. +/// +/// Unconditional includes are de-duplicated by trimmed text across tabs. +/// Returns the hoisted lines and each tab's body with those lines blanked. +fn hoist_leading_preprocessor(contents: &[String]) -> (Vec, Vec) { + let mut seen_includes = HashSet::new(); + let mut hoisted = Vec::new(); + let mut stripped = Vec::with_capacity(contents.len()); + + for (tab, content) in contents.iter().enumerate() { + let source_lines: Vec<&str> = content.lines().collect(); + let mut moved = HashSet::new(); + for leading in leading_preprocessor_region(content) { + let line = source_lines[leading.index]; + let unconditional_include = leading.is_include && leading.depth == 0; + if tab == 0 { + if unconditional_include { + seen_includes.insert(line.trim().to_string()); + } + hoisted.push(line.to_string()); + moved.insert(leading.index); + } else if unconditional_include + && !leading.is_continuation + && !line.trim_end().ends_with('\\') + { + if seen_includes.insert(line.trim().to_string()) { + hoisted.push(line.trim().to_string()); + } + moved.insert(leading.index); + } + } + let body = source_lines + .iter() + .enumerate() + .map(|(i, line)| if moved.contains(&i) { "" } else { *line }) + .collect::>() + .join("\n"); + stripped.push(body); + } + (hoisted, stripped) } /// Extract function prototypes from concatenated .ino source using a C++ parser. @@ -1220,3 +1349,6 @@ fn trim_trailing_spaces(text: &mut String) { #[cfg(test)] mod tests; + +#[cfg(test)] +mod tests_include_hoisting; diff --git a/crates/fbuild-build-engine/src/source_scanner/tests.rs b/crates/fbuild-build-engine/src/source_scanner/tests.rs index 163e0bd34..be1570760 100644 --- a/crates/fbuild-build-engine/src/source_scanner/tests.rs +++ b/crates/fbuild-build-engine/src/source_scanner/tests.rs @@ -5,7 +5,7 @@ use super::*; use std::fs; use tempfile::TempDir; -fn setup_project(src_files: &[(&str, &str)]) -> (TempDir, PathBuf, PathBuf) { +pub(super) fn setup_project(src_files: &[(&str, &str)]) -> (TempDir, PathBuf, PathBuf) { let tmp = TempDir::new().unwrap(); let src_dir = tmp.path().join("src"); let build_dir = tmp.path().join("build"); diff --git a/crates/fbuild-build-engine/src/source_scanner/tests_include_hoisting.rs b/crates/fbuild-build-engine/src/source_scanner/tests_include_hoisting.rs new file mode 100644 index 000000000..8c44a5c1f --- /dev/null +++ b/crates/fbuild-build-engine/src/source_scanner/tests_include_hoisting.rs @@ -0,0 +1,143 @@ +//! FastLED/fbuild#1440: `.ino` include hoisting must never lift an `#include` +//! out of its `#if`. Split from `tests.rs` to keep it under the 1000-LOC gate. + +use super::tests::setup_project; +use super::*; +use std::fs; + +/// Generated `.ino.cpp` for a single-tab sketch, split at its `#line 1`. +fn generated_prelude_and_body(sketch: &str) -> (String, String) { + let (_tmp, src_dir, build_dir) = setup_project(&[("sketch.ino", sketch)]); + let scanner = SourceScanner::new(&src_dir, &build_dir); + let sources = scanner.scan_sketch_sources().unwrap(); + let content = fs::read_to_string(&sources[0]).unwrap(); + let (prelude, body) = content + .split_once("#line 1 \"src/sketch.ino\"\n") + .expect("generated file carries a #line 1 directive"); + (prelude.to_string(), body.to_string()) +} + +#[test] +fn test_conditional_include_stays_inside_its_if() { + // The FastLED Sailboat shape that broke every non-Teensy root build. + let (prelude, body) = generated_prelude_and_body( + "#include \n\n#if defined(FL_IS_TEENSY)\n#include \n#endif\n#include \"fl/ui/ui.h\"\n\nint helper(int x) { return x; }\nvoid setup() {}\nvoid loop() {}\n", + ); + let lines: Vec<&str> = prelude.lines().collect(); + let if_pos = lines + .iter() + .position(|l| *l == "#if defined(FL_IS_TEENSY)") + .expect("the #if moves with its include"); + assert_eq!(lines[if_pos + 1], "#include "); + assert_eq!(lines[if_pos + 2], "#endif"); + // Exactly one Audio.h, and only inside the guard. + assert_eq!(prelude.matches("#include ").count(), 1); + assert!(!body.contains("#include ")); + // Headers after the block still precede the prototypes. + let ui_pos = prelude.find("#include \"fl/ui/ui.h\"").unwrap(); + let proto_pos = prelude + .find("// Auto-generated function prototypes") + .unwrap(); + assert!(ui_pos < proto_pos); +} + +#[test] +fn test_define_before_include_keeps_its_order() { + let (prelude, _body) = generated_prelude_and_body( + "#define FASTLED_CONFIG_KNOB 1\n#include \n\nvoid setup() {}\nvoid loop() {}\n", + ); + let define_pos = prelude.find("#define FASTLED_CONFIG_KNOB 1").unwrap(); + let include_pos = prelude.find("#include ").unwrap(); + assert!( + define_pos < include_pos, + "a #define that configures a header must still precede it" + ); +} + +#[test] +fn test_include_after_first_code_line_stays_in_place() { + let (prelude, body) = generated_prelude_and_body( + "#include \nint counter = 0;\n#include \"late.h\"\n\nvoid setup() {}\nvoid loop() {}\n", + ); + assert!(!prelude.contains("late.h")); + let body_lines: Vec<&str> = body.lines().collect(); + assert_eq!(body_lines[0], "", "the leading include is blanked"); + assert_eq!(body_lines[1], "int counter = 0;"); + assert_eq!(body_lines[2], "#include \"late.h\"", "left on its own line"); +} + +#[test] +fn test_spaced_include_directive_is_recognised() { + let (prelude, body) = + generated_prelude_and_body("# include \nvoid setup() {}\nvoid loop() {}\n"); + assert!(prelude.contains("# include ")); + assert!(!body.contains("include ")); +} + +#[test] +fn test_include_inside_block_comment_is_not_a_directive() { + let (prelude, body) = generated_prelude_and_body( + "/*\n#include \n*/\n#include \nvoid setup() {}\nvoid loop() {}\n", + ); + // The comment moves verbatim, so the commented-out include is still + // inside a comment -- never a bare directive. + let comment_open = prelude.find("/*").unwrap(); + let nope = prelude.find("#include ").unwrap(); + let comment_close = prelude.find("*/").unwrap(); + assert!(comment_open < nope && nope < comment_close); + assert!(!body.contains("Nope.h")); +} + +#[test] +fn test_later_tab_hoists_only_unconditional_includes() { + let (_tmp, src_dir, build_dir) = setup_project(&[ + ( + "main.ino", + "#include \nvoid setup() {}\nvoid loop() {}\n", + ), + ( + "tab.ino", + "#define TAB_ONLY 1\n#include \n#ifdef ARDUINO_TEENSY41\n#include \n#endif\nvoid helper() {}\n", + ), + ]); + let scanner = SourceScanner::new(&src_dir, &build_dir); + let sources = scanner.scan_sketch_sources().unwrap(); + let content = fs::read_to_string(&sources[0]).unwrap(); + let (prelude, rest) = content.split_once("#line 1 \"src/main.ino\"\n").unwrap(); + let (_, tab_body) = rest.split_once("#line 1 \"src/tab.ino\"\n").unwrap(); + + assert!(prelude.contains("#include ")); + // A later tab's #define and guarded include stay in that tab, in order. + assert!(!prelude.contains("TAB_ONLY")); + assert!(!prelude.contains("Audio.h")); + let tab_lines: Vec<&str> = tab_body.lines().collect(); + assert_eq!(tab_lines[0], "#define TAB_ONLY 1"); + assert_eq!(tab_lines[1], "", "the unconditional include is blanked"); + assert_eq!(tab_lines[2], "#ifdef ARDUINO_TEENSY41"); + assert_eq!(tab_lines[3], "#include "); + assert_eq!(tab_lines[4], "#endif"); +} + +#[test] +fn test_later_tab_continued_include_stays_whole() { + // A `\`-continued include in a later tab must not be split: hoisting its + // final line alone left a bare `` in the prelude and a dangling + // `#include \` in the body. + let (_tmp, src_dir, build_dir) = setup_project(&[ + ( + "main.ino", + "#include \nvoid setup() {}\nvoid loop() {}\n", + ), + ("tab.ino", "#include \\\n\nvoid helper() {}\n"), + ]); + let scanner = SourceScanner::new(&src_dir, &build_dir); + let sources = scanner.scan_sketch_sources().unwrap(); + let content = fs::read_to_string(&sources[0]).unwrap(); + let (prelude, rest) = content.split_once("#line 1 \"src/main.ino\"\n").unwrap(); + let (_, tab_body) = rest.split_once("#line 1 \"src/tab.ino\"\n").unwrap(); + + assert!(!prelude.contains("Wire.h")); + let tab_lines: Vec<&str> = tab_body.lines().collect(); + assert_eq!(tab_lines[0], "#include \\"); + assert_eq!(tab_lines[1], ""); +}