FileWalker::flat_next(), pop() and depth() are silent no-ops on every document and archive filesystem — flat_next() in particular turns the loop it exists for into an infinite one.
Where
VirtualFilesystem's walker in src/odr/internal/common/filesystem.cpp, which is what Document::as_filesystem() and Archive::as_filesystem() hand out:
[[nodiscard]] std::uint32_t depth() const override {
return 0; // TODO
}
void pop() override {
// TODO
}
void flat_next() override {
// TODO
}
SystemFilesystem's walker implements all three properly (m_iterator.depth() etc.), so this only affects the in-memory filesystems — i.e. every document and every archive.
Why it matters
Nothing in the public header says these three do not work. src/odr/filesystem.hpp declares depth(), pop() and flat_next() alongside next() and end() with no distinction, so a caller has no way to know that two of them do nothing and one of them is a trap:
// the loop flat_next() exists for — never terminates
while (!walker.end()) {
walker.flat_next();
}
flat_next() does not advance the iterator, so end() never becomes true. pop() has the same shape of problem in a while (walker.depth() > 0) walker.pop(); loop, made worse by depth() always being 0.
This is reachable from every binding that exposes FileWalker — it is how I hit it while writing the Objective-C bindings.
Reproduction
Against test/data/input/odr-public/odt/about.odt:
const Filesystem fs = odr::open(path).as_document_file().document().as_filesystem();
const FileWalker w = fs.file_walker("/");
const std::string start = w.path();
for (int i = 0; i < 5 && !w.end(); ++i) {
w.flat_next();
}
// start == w.path(), and !w.end() — the walker has not moved
const FileWalker w2 = fs.file_walker("/");
w2.next();
const std::string before = w2.path();
w2.pop();
// before == w2.path()
const FileWalker w3 = fs.file_walker("/");
while (!w3.end()) {
// depth() is 0 for every entry, including
// "/Configurations2/accelerator/current.xml"
w3.next();
}
Observed:
flatNext x5: path unchanged = true | still not at end = true
pop: path unchanged = true
depth: distinct values = [0]
deepest path = /Configurations2/accelerator/current.xml
Possible resolutions
VirtualFileWalker iterates a flat std::map<AbsPath, ...>, so all three are implementable from the path alone: depth() is the number of path components below the walker's root, flat_next() advances past every entry whose path has the current entry as a prefix, and pop() does the same for the current entry's parent.
If they are not going to be implemented, throwing UnsupportedOperation would at least match the "fail fast rather than silently degrade" rule in AGENTS.md — a caller can handle an exception, but cannot detect a no-op.
There is a smaller, separate oddity in the same area: exists() and is_directory() return false for / and for /Configurations2, while the walker reports /Configurations2/floater as a directory. VirtualFilesystem only knows the directories that exist as explicit map entries, so intermediate ones are invisible. Happy to split that out if you would rather keep this issue to the walker.
FileWalker::flat_next(),pop()anddepth()are silent no-ops on every document and archive filesystem —flat_next()in particular turns the loop it exists for into an infinite one.Where
VirtualFilesystem's walker insrc/odr/internal/common/filesystem.cpp, which is whatDocument::as_filesystem()andArchive::as_filesystem()hand out:SystemFilesystem's walker implements all three properly (m_iterator.depth()etc.), so this only affects the in-memory filesystems — i.e. every document and every archive.Why it matters
Nothing in the public header says these three do not work.
src/odr/filesystem.hppdeclaresdepth(),pop()andflat_next()alongsidenext()andend()with no distinction, so a caller has no way to know that two of them do nothing and one of them is a trap:flat_next()does not advance the iterator, soend()never becomes true.pop()has the same shape of problem in awhile (walker.depth() > 0) walker.pop();loop, made worse bydepth()always being0.This is reachable from every binding that exposes
FileWalker— it is how I hit it while writing the Objective-C bindings.Reproduction
Against
test/data/input/odr-public/odt/about.odt:Observed:
Possible resolutions
VirtualFileWalkeriterates a flatstd::map<AbsPath, ...>, so all three are implementable from the path alone:depth()is the number of path components below the walker's root,flat_next()advances past every entry whose path has the current entry as a prefix, andpop()does the same for the current entry's parent.If they are not going to be implemented, throwing
UnsupportedOperationwould at least match the "fail fast rather than silently degrade" rule inAGENTS.md— a caller can handle an exception, but cannot detect a no-op.There is a smaller, separate oddity in the same area:
exists()andis_directory()returnfalsefor/and for/Configurations2, while the walker reports/Configurations2/floateras a directory.VirtualFilesystemonly knows the directories that exist as explicit map entries, so intermediate ones are invisible. Happy to split that out if you would rather keep this issue to the walker.