Skip to content

Parser.Dep: fix expand_directories filtering explicit cmd-line files#4144

Closed
tahina-pro wants to merge 1 commit intoFStarLang:fstar2from
tahina-pro:_taramana_fstar2_depend_relative
Closed

Parser.Dep: fix expand_directories filtering explicit cmd-line files#4144
tahina-pro wants to merge 1 commit intoFStarLang:fstar2from
tahina-pro:_taramana_fstar2_depend_relative

Conversation

@tahina-pro
Copy link
Copy Markdown
Member

expand_directories was unconditionally removing .fst files when a corresponding .fsti existed, even for files explicitly listed on the command line. This caused build_map to retain absolute paths from include-path discovery instead of the user-provided relative paths, making .depend targets switch from relative to absolute paths.

Fix: only apply the .fst/.fsti filtering to files discovered through directory expansion, preserving explicitly listed command-line files.

(Generated by GitHub Copilot CLI with Claude Opus 4.6. This PR works well with EverParse fstar2 as of project-everest/everparse@68c594f )

expand_directories was unconditionally removing .fst files when a
corresponding .fsti existed, even for files explicitly listed on the
command line. This caused build_map to retain absolute paths from
include-path discovery instead of the user-provided relative paths,
making .depend targets switch from relative to absolute paths.

Fix: only apply the .fst/.fsti filtering to files discovered through
directory expansion, preserving explicitly listed command-line files.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Member

@mtzguido mtzguido left a comment

Choose a reason for hiding this comment

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

I'm confused about this recursive inclusion, I missed it during the last PR on Dep. We already have fstar.include that allows some controlled recursion, though it requires manual input. IIUC now fstar.exf d will recursively traverse d and ignore fstar.include. Should we just do that always? Or default to it if there's no fstar.include instead of defaulting to "no subdirectories"?

I also don't understand why there's any fst filtering at all.

The test seems to not be doing anything interesting and it's also not wired in, we need SUBDIRS += dep or similar in tests/Makefile.

@@ -0,0 +1,3 @@
module A
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This test is not wired in (tests/dep/ is introduced by this PR). Nor does it check anything related to this particular issue?

@mtzguido
Copy link
Copy Markdown
Member

mtzguido commented Apr 2, 2026

Superseded by #4156

@mtzguido mtzguido closed this Apr 2, 2026
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.

2 participants