Yeah, maybe that was the original logic. Probably would make sense to not worry about stopping warnings but just pipe them to a log.
@seatbelts: sorry we were over confident about that solution, it looks like our include order might be triggering this problem even though the error itself looks nonsensical (“limits.h” is on the search path).
We probably need to look at our include order more broadly
Yes, the isystem was originally introduced to suppress warnings from Boost and Eigen. A couple relevant points:
since we’ve used it, Boost and Eigen have gotten a lot better with their warnings. I don’t think they’re fully gone yet, but it’s close
it’s not well documented by any of the compilers, but the way includes are handled by isystem and I differ from compiler to compiler and I’ve even seen it differ version to version within a compiler
Yes, we should look at include order more broadly, but I don’t think there’s going to be a neat way to solve this problem. Since it differs within versions of a compiler, we can only set up defaults that work across most situations and then special-case all the places where it fails. Right now, we’re not doing that.
If I recall, the place where this really messes up (besides for the compiler errors you’re seeing) is with system installs of Boost. The includes don’t pick out the one in the math library if you’re not careful. I thought I wrote this all out somewhere… let me look for it.
I can’t find the old thread / pull request comment / issue comment / whatever I think I wrote.