Skip to content

Commit d892eec

Browse files
committed
Reapply: Make header inclusion order from umbrella dirs deterministic
Sort the headers by name before adding the includes in collectModuleHeaderIncludes. This makes the include order for building umbrellas deterministic across different filesystems and also guarantees that the ASTWriter always dump top headers in the same order. There's currently no good way to test for this behavior. This was first introduced in r289478 and reverted few times because of ASANifed test failures on open source bots (both from LLVM and Swift). Finally reproduced the problem in a Linux machine and use std::sort as a fix, since we are not dealing with POD-like types. rdar://problem/28116411
1 parent 01d2a01 commit d892eec

File tree

1 file changed

+16
-3
lines changed

1 file changed

+16
-3
lines changed

clang/lib/Frontend/FrontendAction.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ static std::error_code collectModuleHeaderIncludes(
364364
llvm::sys::path::native(UmbrellaDir.Entry->getName(), DirNative);
365365

366366
llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
367+
SmallVector<std::pair<std::string, const FileEntry *>, 8> Headers;
367368
for (llvm::vfs::recursive_directory_iterator Dir(FS, DirNative, EC), End;
368369
Dir != End && !EC; Dir.increment(EC)) {
369370
// Check whether this entry has an extension typically associated with
@@ -394,13 +395,25 @@ static std::error_code collectModuleHeaderIncludes(
394395
++It)
395396
llvm::sys::path::append(RelativeHeader, *It);
396397

397-
// Include this header as part of the umbrella directory.
398-
Module->addTopHeader(*Header);
399-
addHeaderInclude(RelativeHeader, Includes, LangOpts, Module->IsExternC);
398+
std::string RelName = RelativeHeader.c_str();
399+
Headers.push_back(std::make_pair(RelName, *Header));
400400
}
401401

402402
if (EC)
403403
return EC;
404+
405+
// Sort header paths and make the header inclusion order deterministic
406+
// across different OSs and filesystems.
407+
llvm::sort(Headers.begin(), Headers.end(), [](
408+
const std::pair<std::string, const FileEntry *> &LHS,
409+
const std::pair<std::string, const FileEntry *> &RHS) {
410+
return LHS.first < RHS.first;
411+
});
412+
for (auto &H : Headers) {
413+
// Include this header as part of the umbrella directory.
414+
Module->addTopHeader(H.second);
415+
addHeaderInclude(H.first, Includes, LangOpts, Module->IsExternC);
416+
}
404417
}
405418

406419
// Recurse into submodules.

0 commit comments

Comments
 (0)