[PATCH] D89488: FileManager: Shrink FileEntryRef to the size of a pointer

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.


================
Comment at: clang/include/clang/Basic/FileManager.h:101
+  /// Type used in the StringMap.
+  using MapEntry = llvm::StringMapEntry<llvm::ErrorOr<MapValue>>;
+
----------------
thakis wrote:
> thakis wrote:
> > It looks like this is too clever for gcc5.3: https://bugs.chromium.org/p/chromium/issues/detail?id=1143022
> > 
> > Ideas on how to work around that?
> Looks like the following might do it. I'll finish building locally and push this if it works:
> 
> ```
> $ git diff
> diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
> index d27b4260cca..12848f42770 100644
> --- a/clang/include/clang/Basic/FileManager.h
> +++ b/clang/include/clang/Basic/FileManager.h
> @@ -107,7 +107,9 @@ public:
>      /// VFSs that use external names. In that case, the \c FileEntryRef
>      /// returned by the \c FileManager will have the external name, and not the
>      /// name that was used to lookup the file.
> -    llvm::PointerUnion<FileEntry *, const MapEntry *> V;
> +    // The second type is really a `const MapEntry *`, but that confuses gcc5.3.
> +    // Once that's no longer supported, change this back.
> +    llvm::PointerUnion<FileEntry *, const void *> V;
>  
>      MapValue() = delete;
>      MapValue(FileEntry &FE) : V(&FE) {}
> diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
> index d26ead4a5b0..4d84c3102ec 100644
> --- a/clang/lib/Basic/FileManager.cpp
> +++ b/clang/lib/Basic/FileManager.cpp
> @@ -215,7 +215,7 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
>      FileEntryRef::MapValue Value = *SeenFileInsertResult.first->second;
>      if (LLVM_LIKELY(Value.V.is<FileEntry *>()))
>        return FileEntryRef(*SeenFileInsertResult.first);
> -    return FileEntryRef(*Value.V.get<const FileEntryRef::MapEntry *>());
> +    return FileEntryRef(*reinterpret_cast<const FileEntryRef::MapEntry *>(Value.V.get<const void *>()));
>    }
>  
>    // We've not seen this before. Fill it in.
> @@ -347,7 +347,7 @@ FileManager::getVirtualFile(StringRef Filename, off_t Size,
>      FileEntry *FE;
>      if (LLVM_LIKELY(FE = Value.V.dyn_cast<FileEntry *>()))
>        return FE;
> -    return &FileEntryRef(*Value.V.get<const FileEntryRef::MapEntry *>())
> +    return &FileEntryRef(*reinterpret_cast<const FileEntryRef::MapEntry *>(Value.V.get<const void *>()))
>                  .getFileEntry();
>    }
>  
> ```
Thanks Nico!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89488