[PATCH] D142113: [clang][nfc] refactor Module::Header to use OptionalFileEntryRef

Jan Svoboda via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 19 09:25:51 PST 2023


jansvoboda11 added inline comments.


================
Comment at: clang/lib/Frontend/FrontendAction.cpp:424
       // Include this header as part of the umbrella directory.
       Module->addTopHeader(H.second);
       addHeaderInclude(H.first, Includes, LangOpts, Module->IsExternC);
----------------
Debian CI is complaining about this:

```
error: no viable conversion from 'clang::CustomizableOptional<clang::FileEntryRef>' to 'const clang::FileEntry *'
```

You might need to refactor `Module::addTopHeader()` too, or use `OptionalFileEntryRefDegradesToFileEntryPtr` in the local `Headers` variable.


================
Comment at: clang/lib/Lex/ModuleMap.cpp:2514
+      if (auto FE = SourceMgr.getFileManager().getOptionalFileRef(I->path())) {
         Module::Header Header = {"", std::string(I->path()), *FE};
         Headers.push_back(std::move(Header));
----------------
Does it make sense to unwrap the optional here? It'll be instantly wrapped in `OptionalFileEntryRefDegradesToFileEntryPtr` anyways.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:1950
     Module::Header H = {std::string(key.Filename), "",
-                        *FileMgr.getFile(Filename)};
+                        *FileMgr.getOptionalFileRef(Filename)};
     ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
----------------
Does it make sense to unwrap the optional here? It'll be instantly wrapped in `OptionalFileEntryRefDegradesToFileEntryPtr` anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142113



More information about the cfe-commits mailing list