[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