Skip to content

Resolving always-false conditional making some compilers miss for preprocessor checks - #2824

Open
Joldiges wants to merge 1 commit into
mozilla:mainfrom
Joldiges:upstream/fix-line-directive
Open

Resolving always-false conditional making some compilers miss for preprocessor checks#2824
Joldiges wants to merge 1 commit into
mozilla:mainfrom
Joldiges:upstream/fix-line-directive

Conversation

@Joldiges

@Joldiges Joldiges commented Aug 27, 2026

Copy link
Copy Markdown

Standard #line directives were never recognized while collecting included
files for preprocessor-cache manifests because the parser compared four bytes
with the five-byte line prefix.

The existing line
|| (&slice[1..5] == b"line "))
Was comparing 4 bytes against 5 causing this conditional to never hit.

This change was primarily made to fix Tasking Compiler runs.

GitHub copilot was used for the debugging and resolution of this issue.

Recognize standard #line directives when collecting included files for preprocessor cache manifests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Joldiges
Joldiges marked this pull request as ready for review August 27, 2026 09:30
Copilot AI lite review requested due to automatic review settings August 27, 2026 09:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes preprocessor-cache manifest collection for C/C++ preprocessed output by correctly recognizing standard #line directives (previously the condition was always false due to a slice-length mismatch), which could cause cache misses for some compilers (notably Tasking).

Changes:

  • Correct #line directive detection in process_preprocessed_file by switching to a proper prefix check (starts_with(b"line ")).
  • Add a unit test validating that #line directives cause the referenced header to be recorded and hashed in included_files.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov-commenter

codecov-commenter commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.74%. Comparing base (8ab3926) to head (adbe9b8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2824      +/-   ##
==========================================
+ Coverage   73.71%   73.74%   +0.02%     
==========================================
  Files          72       72              
  Lines       37932    37975      +43     
==========================================
+ Hits        27963    28006      +43     
  Misses       9969     9969              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants