[clang] use FileEntryRef for SUBMODULE_TOPHEADER
Needs ReviewPublic
Actions

Authored by rmaz on Jan 27 2023, 7:31 AM.

Details

Reviewers

drodriguez
jansvoboda11

Summary

Refactor a Module's TopHeaders to use FileEntryRef. This will keep
the paths serialized in a module the same as the ones used to look
up the header initially.

Diff Detail

Repository: rG LLVM Github Monorepo

Unit TestsFailed
View All

	Time	Test
	3,650 ms	libcxx CI Modules > llvm-libc++-shared-cfg-in.libcxx/algorithms/specialized_algorithms/special_mem_concepts::nothrow_sentinel_for.compile.pass.cpp Script: -- : 'COMPILED WITH'; /home/libcxx-builder/.buildkite-agent/builds/b44fbc847e23-1/llvm-project/libcxx-ci/install/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/b44fbc847e23-1/llvm-project/libcxx-ci/libcxx/test/libcxx/algorithms/specialized.algorithms/special.mem.concepts/nothrow_sentinel_for.compile.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/b44fbc847e23-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/b44fbc847e23-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/b44fbc847e23-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -fsyntax-only

Event Timeline

rmaz created this revision.Jan 27 2023, 7:31 AM

Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2023, 7:31 AM

Herald added a subscriber: arphaman. · View Herald Transcript

rmaz requested review of this revision.Jan 27 2023, 7:31 AM

Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2023, 7:31 AM

Herald added a subscriber: cfe-commits. · View Herald Transcript

rmaz added reviewers: drodriguez, jansvoboda11.Jan 27 2023, 7:33 AM

jansvoboda11 added inline comments.Jan 27 2023, 9:16 AM

clang/lib/Basic/Module.cpp
271–272	If we assert that `File` is not empty right away, why take `OptionalFileEntryRef` instead of `FileEntryRef`?
clang/lib/Lex/ModuleMap.cpp
663	How much work would be to refactor `File` to be `FileEntryRef`? Would be good if we didn't need to resort to `getLastRef()`.

Harbormaster completed remote builds in B210360: Diff 492754.Jan 27 2023, 9:32 AM

Use FileEntryRef for AddTopHeader()

rmaz marked an inline comment as done.Jan 27 2023, 10:07 AM

rmaz added inline comments.

clang/lib/Lex/ModuleMap.cpp
663	It is unfortunately a fair bit. I spent some time on it but hit a roadblock with the FileManagers VirtualFileEntries: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/FileManager.h#L70-L71 We would require this to be refactored to FileEntry too, as it is used here to provide a UID mapping to FileEntry pointers which needs to be changed to refs: https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/FileManager.cpp#L614-L633 I can put up a follow on diff once this one is shipped if preferred?

jansvoboda11 added inline comments.Jan 27 2023, 12:08 PM

clang/lib/Lex/ModuleMap.cpp
663	I was able to refactor the UID mapping to use `FileEntryRef` in D142780. Given that, I think it would be preferable to propagate these into `findOrCreateModuleForHeaderInUmbrellaDir()` and remove `getLastRef()`.