[PATCH] D100668: [ADT] Add STLForwardCompat.h and llvm::disjunction

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.


================
Comment at: llvm/include/llvm/ADT/STLForwardCompat.h:36-37
+template <typename B1, typename... Bn>
+struct conjunction<B1, Bn...>
+    : std::conditional<bool(B1::value), conjunction<Bn...>, B1>::type {};
+
----------------
dblaikie wrote:
> Hmm, seems this isn't actually conforming implementation (the standard one says that it short-circuits - doesn't try to evaluate later expressions if an early one is true/false - and that the type returned by std::conditional is the type of that last condition that proved the rule, I think?). But probably close enough since it's already committed.
My understanding of the definition of `std::conditional` led me to believe this definition does satisfy the "short-circuiting" property, and I tried to include at least a (potentially incomplete?) trivial test of that using an `incomplete_type` following an element that proves the rule of the predicate (i.e. `false` for `conjunction`, `true` for `disjunction`)


================
Comment at: llvm/unittests/ADT/STLForwardCompatTest.cpp:22-30
+  EXPECT_TRUE((llvm::conjunction<>::value));
+  EXPECT_FALSE((llvm::conjunction<std::false_type>::value));
+  EXPECT_TRUE((llvm::conjunction<std::true_type>::value));
+  EXPECT_FALSE((llvm::conjunction<std::false_type, incomplete_type>::value));
+  EXPECT_FALSE((llvm::conjunction<std::false_type, std::true_type>::value));
+  EXPECT_FALSE((llvm::conjunction<std::true_type, std::false_type>::value));
+  EXPECT_TRUE((llvm::conjunction<std::true_type, std::true_type>::value));
----------------
dblaikie wrote:
> Was there no existing testing of conjunction? (ie: something that should be deleted as the testing is added/moved here?)
Not sure, I assume it just wasn't noticed in review? There are tests of it indirectly via `Any`, see https://reviews.llvm.org/D48807


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100668