[PATCH] D87187: [Driver] Perform Linux distribution detection just once
Alexandre Ganea via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 25 09:25:30 PDT 2020
aganea accepted this revision.
aganea added a comment.
Makes sense, you cache the commonly taken path, but not the (very uncommon) VFS codepath.
LGTM, with some minor comments. Thanks again!
================
Comment at: clang/include/clang/Driver/Distro.h:117
- bool IsOpenSUSE() const {
- return DistroVal == OpenSUSE;
- }
+ bool IsOpenSUSE() const { return DistroVal == OpenSUSE; }
----------------
Please leave out code that doesn't participate in the patch. You can always commit NFC patches afterward with just `clang-format` changes if you wish. It's fine though if you move code around.
================
Comment at: clang/lib/Driver/Distro.cpp:225
+ static Distro::DistroType LinuxDistro = Distro::UninitializedDistro;
+ static llvm::once_flag DistroDetectFlag;
+ // If we're backed by a real file system, perform
----------------
You can leave out the `once_flag`/`call_once` and simply apply Reid's suggestion, since they are equivalent (static initialization is thread safe [1]):
```
if (onRealFS) {
static Distro::DistroType LinuxDistro = DetectDistro(VFS);
return LinuxDistro;
}
```
[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3797.pdf, section 6.7.4 "If control enters the declaration concurrently while the variable is being initialized, the concurrent execution shall wait for completion of the initialization"
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87187/new/
https://reviews.llvm.org/D87187
More information about the cfe-commits
mailing list