[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 1 10:44:18 PDT 2022


dexonsmith added a comment.

In D135220#3870991 <https://reviews.llvm.org/D135220#3870991>, @hans wrote:

> One thought, which I'm not sure is relevant, is that this is only observable for headers which are included more than once, which is rare because normally there are include guards (or pragma once). `isFileMultipleIncludeGuarded` isn't tracked by the FileManager, but otherwise maybe that could have been a useful heuristic: don't merge header files without include guards?

Maybe that heuristic would work a level up, in SourceManager.

- If multiple-include-guarded, create one FileID per FileEntry.
- Else, create one FileID per FileEntryRef.

Although it gets complicated with language features like `#import` in Objective-C, where textual inclusion is implicitly multiple-include-guarded. Consider a file included both using `#include` (not guarded) and as `#import` (guarded).

And I'm not sure we really want to split the FileIDs... that seems like a potential performance regression. Instead, I think we just want to track (at the use site) how the file was referenced.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135220/new/

https://reviews.llvm.org/D135220



More information about the cfe-commits mailing list