Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
210 changes: 171 additions & 39 deletions crates/fbuild-build-engine/src/source_scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,17 +349,14 @@ impl SourceScanner {
})
.collect::<fbuild_core::Result<Vec<_>>>()?;

// 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<String> = 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
Expand Down Expand Up @@ -787,44 +784,176 @@ fn walk_sources(dir: &Path) -> Vec<PathBuf> {
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<String> {
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<LeadingLine> {
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::<Vec<_>>()
.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 <Audio.h> #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<String>, Vec<String>) {
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::<Vec<_>>()
.join("\n");
stripped.push(body);
}
(hoisted, stripped)
}

/// Extract function prototypes from concatenated .ino source using a C++ parser.
Expand Down Expand Up @@ -1220,3 +1349,6 @@ fn trim_trailing_spaces(text: &mut String) {

#[cfg(test)]
mod tests;

#[cfg(test)]
mod tests_include_hoisting;
2 changes: 1 addition & 1 deletion crates/fbuild-build-engine/src/source_scanner/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <FastLED.h>\n\n#if defined(FL_IS_TEENSY)\n#include <Audio.h>\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 <Audio.h>");
assert_eq!(lines[if_pos + 2], "#endif");
// Exactly one Audio.h, and only inside the guard.
assert_eq!(prelude.matches("#include <Audio.h>").count(), 1);
assert!(!body.contains("#include <Audio.h>"));
// 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 <FastLED.h>\n\nvoid setup() {}\nvoid loop() {}\n",
);
let define_pos = prelude.find("#define FASTLED_CONFIG_KNOB 1").unwrap();
let include_pos = prelude.find("#include <FastLED.h>").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 <FastLED.h>\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 <FastLED.h>\nvoid setup() {}\nvoid loop() {}\n");
assert!(prelude.contains("# include <FastLED.h>"));
assert!(!body.contains("include <FastLED.h>"));
}

#[test]
fn test_include_inside_block_comment_is_not_a_directive() {
let (prelude, body) = generated_prelude_and_body(
"/*\n#include <Nope.h>\n*/\n#include <FastLED.h>\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 <Nope.h>").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 <FastLED.h>\nvoid setup() {}\nvoid loop() {}\n",
),
(
"tab.ino",
"#define TAB_ONLY 1\n#include <Wire.h>\n#ifdef ARDUINO_TEENSY41\n#include <Audio.h>\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 <Wire.h>"));
// 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 <Audio.h>");
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 `<Wire.h>` in the prelude and a dangling
// `#include \` in the body.
let (_tmp, src_dir, build_dir) = setup_project(&[
(
"main.ino",
"#include <FastLED.h>\nvoid setup() {}\nvoid loop() {}\n",
),
("tab.ino", "#include \\\n<Wire.h>\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], "<Wire.h>");
}
Loading