royjacobson added a comment. In D130689#3705145 <https://reviews.llvm.org/D130689#3705145>, @thieta wrote:
> In D130689#3705131 <https://reviews.llvm.org/D130689#3705131>, @royjacobson > wrote: > >> This seems to have been more disruptive than expected, since an existing >> CMakeCache.txt can make LLVM compile in previous C++14 configuration. This >> seems to make some of the bots fail in a way that makes the patches making >> use of C++17 features seem at fault. >> >> See: >> https://github.com/llvm/llvm-project/commit/ede96de751224487aea122af8bfb4e82bc54840b#commitcomment-80507826 >> https://reviews.llvm.org/rG32fd0b7fd5ab >> >> How would you feel about adding something like >> >> #if defined(__cplusplus) && __cplusplus < 201703L >> #error "LLVM requires at least C++17" >> #endif >> >> to some central header, to make this switch more visible? > > I am not opposed to that directly. But this seems a bit dangerous where bots > retain the cmakecache - there must be other cases where we can't really > protect in this way. > > Another approach would be to unset CMAKE_CXX_STANDARD if it's below 17 in > cmake directly. > > But in general - I am not a huge fan of CI / bots trying to keep the cache > around - many weird issues can arise from this. This affects people on their work branches as well, and it's not obvious that it's a configuration error and not a broken master. The CMake approach sounds cleaner to me, but I don't know CMake well enough to do it - if you could post a follow up patch I think it would be quite helpful. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130689/new/ https://reviews.llvm.org/D130689 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
