kadircet marked an inline comment as done.
kadircet added inline comments.

================
Comment at: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:14
+#include "llvm/ADT/STLFunctionalExtras.h"
+#include <variant>
+
----------------
kadircet wrote:
> sammccall wrote:
> > This is awful but: as mentioned on another patch, we can't actually use 
> > variant. AFAIK all C++17 *language* features are available, but LLVM 
> > supports apple clang 9.3, which doesn't provide <variant> at all.
> > 
> > There's an IntrusiveVariant in review somewhere but seems stalled (and 
> > honestly the Intrusive part makes it pretty ugly to use).
> > 
> > I think for now our best option is a Kind enum and some kind of uintptr_t 
> > storage :-(
> right, i've seen that comment as well and wanted to pile up there. there are 
> already uses of `<variant>` in LLVM today, TableGen, DenseMap, CodeGen, 
> flang, pseudo, MLIR and lldb are some examples of big components. I don't see 
> what they're doing to compile those today.
as mentioned in 
https://discourse.llvm.org/t/rfc-bump-minimal-requirements-apple-clang-9-3-10-0-0-before-4th-tue-in-january/66156/7?u=kadircet,
 LLVM is already enforcing apple clang 10.0.0 effectively (and hopefully 
documentation will also change soon).

hence i am moving forward with this change. in the unlikely event of community 
taking a different approach (like downgrading the minimum requirements), i am 
more than happy to get rid of the std::variant usage here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136293

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to