base::mac::IsOSSierra() -> base::mac::IsOS10_12(), etc.

base/mac/mac_util.h

Index: base/mac/mac_util.h
diff --git a/base/mac/mac_util.h b/base/mac/mac_util.h
index 84948f7ce89fc8b35d87be7d052fa50254afc879..b403d34b327c47b441d7c8685facb69924f9a916 100644
--- a/base/mac/mac_util.h
+++ b/base/mac/mac_util.h
@@ -108,85 +108,75 @@ BASE_EXPORT bool WasLaunchedAsHiddenLoginItem();
 // an error, or true otherwise.
 BASE_EXPORT bool RemoveQuarantineAttribute(const FilePath& file_path);
 
+namespace internal {
+
+// Returns the running system's Mac OS X minor version. This is the |y| value
+// in 10.y or 10.y.z.
+BASE_EXPORT int MacOSXMinorVersion();
+
+// Class to compare the running system's Mac OS X minor version with a known
+// one. When ID is a MAC_OS_X_VERSION_10_* macro from AvailabilityMacros.h,
+// some tests may compile to constants depending on deployment target
+// (-mmacosx-version-min). For instance, OSVersion<9>::IsAtLeast() is always
+// true with a deployment target of 10.10. ID can also be 0 to support an OS
+// version that's newer than the build SDK.
+template <int V, int ID>
+class OSVersion {
+ public:
+  static constexpr bool Equals() {
+    return !DeploymentTargetIsGreater() && MacOSXMinorVersion() == V;
+  }
+
+  static constexpr bool IsAtLeast() {
+    return DeploymentTargetIsGreater() || MacOSXMinorVersion() >= V;

[Mark Mentovai, 2016/08/26 15:27:26]

This doesn’t short-circuit every case that it could.

(Hint: this looked suspicious because logically, you can’t implement both >= and
<= on the same underlying >.)

With DT = 10, OSVersion<10>::DeploymentTargetIsGreater() is constexpr false, so
OSVersion<10>::IsAtLeast() has to evaluate MacOSXMinorVersion(). But with DT =
10, OSVersion<10>::DeploymentTargetIsGreater() must be true, no need to evaluate
MacOSXMinorVersion().

[Sidney San Martín, 2016/08/28 21:27:17]

Good catch, thanks. Let me know if this new patch is easier to reason about!

+  }
+
+  static constexpr bool IsAtMost() {
+    return !DeploymentTargetIsGreater() && MacOSXMinorVersion() <= V;
+  }
+
+ private:
+  // Is the minimum OS version greater than this one?
+  static constexpr bool DeploymentTargetIsGreater() {
+    return ID && MAC_OS_X_VERSION_MIN_REQUIRED > ID;
+  }
+};
+}  // namespace internal
+
 // Run-time OS version checks. Use these instead of
-// base::SysInfo::OperatingSystemVersionNumbers. Prefer the "OrEarlier" and
-// "OrLater" variants to those that check for a specific version, unless you
+// base::SysInfo::OperatingSystemVersionNumbers. Prefer the "IsAtLeastOS" and
+// "IsAtMostOS" variants to those that check for a specific version, unless you
 // know for sure that you need to check for a specific version.
 
-// Mavericks is OS X 10.9, Darwin 13.
-BASE_EXPORT bool IsOSMavericks();
-
-// Yosemite is OS X 10.10, Darwin 14.
-BASE_EXPORT bool IsOSYosemite();
-BASE_EXPORT bool IsOSYosemiteOrEarlier();
-BASE_EXPORT bool IsOSYosemiteOrLater();
-
-// El Capitan is OS X 10.11, Darwin 15.
-BASE_EXPORT bool IsOSElCapitan();
-BASE_EXPORT bool IsOSElCapitanOrEarlier();
-BASE_EXPORT bool IsOSElCapitanOrLater();
-
-// Sierra is macOS 10.12, Darwin 16.
-BASE_EXPORT bool IsOSSierra();
-BASE_EXPORT bool IsOSSierraOrLater();
-
-// This should be infrequently used. It only makes sense to use this to avoid
-// codepaths that are very likely to break on future (unreleased, untested,
-// unborn) OS releases, or to log when the OS is newer than any known version.
-BASE_EXPORT bool IsOSLaterThanSierra_DontCallThis();
-
-// Inline functions that are redundant due to version ranges being mutually-
-// exclusive.
-inline bool IsOSYosemiteOrEarlier() { return !IsOSElCapitanOrLater(); }
-inline bool IsOSElCapitanOrEarlier() { return !IsOSSierraOrLater(); }
-
-// When the deployment target is set, the code produced cannot run on earlier
-// OS releases. That enables some of the IsOS* family to be implemented as
-// constant-value inline functions. The MAC_OS_X_VERSION_MIN_REQUIRED macro
-// contains the value of the deployment target.
-
-#if defined(MAC_OS_X_VERSION_10_9) && \
-    MAC_OS_X_VERSION_MIN_REQUIRED > MAC_OS_X_VERSION_10_9
-#define BASE_MAC_MAC_UTIL_H_INLINED_GT_10_9
-inline bool IsOSMavericks() { return false; }
-#endif
-
-#if defined(MAC_OS_X_VERSION_10_10) && \
-    MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_10
-#define BASE_MAC_MAC_UTIL_H_INLINED_GE_10_10
-inline bool IsOSYosemiteOrLater() { return true; }
-#endif
+#define DECLARE_IS_OS_FUNCS(V, ID)                                      \

[Mark Mentovai, 2016/08/26 15:27:26]

This defines them too, so DEFINE instead of DECLARE?

[Sidney San Martín, 2016/08/28 21:27:17]

Done.

+  const auto IsOS10_##V = internal::OSVersion<V, ID>::Equals;           \
+  const auto IsAtLeastOS10_##V = internal::OSVersion<V, ID>::IsAtLeast; \

[Mark Mentovai, 2016/08/26 15:27:26]

We would not otherwise find it useful to implement IsAtLeastOS10_9 at all, but
not defining it is harder than defining it with the macros laid out as they are
here.

[Sidney San Martín, 2016/08/28 21:27:17]

As discussed, I'll try to ditch the 10.9 versions completely.

+  const auto IsAtMostOS10_##V = internal::OSVersion<V, ID>::IsAtMost;
 
-#if defined(MAC_OS_X_VERSION_10_10) && \
-    MAC_OS_X_VERSION_MIN_REQUIRED > MAC_OS_X_VERSION_10_10
-#define BASE_MAC_MAC_UTIL_H_INLINED_GT_10_10
-inline bool IsOSYosemite() { return false; }
-#endif
+DECLARE_IS_OS_FUNCS(9, MAC_OS_X_VERSION_10_9)
+DECLARE_IS_OS_FUNCS(10, MAC_OS_X_VERSION_10_10)
 
-#if defined(MAC_OS_X_VERSION_10_11) && \
-    MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_11
-#define BASE_MAC_MAC_UTIL_H_INLINED_GE_10_11
-inline bool IsOSElCapitanOrLater() { return true; }
+// Remove these guards when we bump our SDK version
+#if defined(MAC_OS_X_VERSION_10_11)
+DECLARE_IS_OS_FUNCS(11, MAC_OS_X_VERSION_10_11)
+#else
+DECLARE_IS_OS_FUNCS(11, 0)

[Mark Mentovai, 2016/08/26 15:27:26]

Since you need to do this #if thing, maybe you can simplify the logic above by
getting rid of the 0 special case and writing 101100 here.

Alternatively, fudge the whole thing by getting rid of the ID template parameter
and basing it all on V in the template:

V >= 10 ? 100000 + V * 100 : 1000 + V * 10;

and then having some static_asserts to check that (if defined) these macros are
really 1090, 101000, 101100.

[Sidney San Martín, 2016/08/28 21:27:17]

Done. The static_asserts and surrounding #ifdef guards almost double the length,
so I'd still be a fan of moving them to a test.

An example of this is base/template_util_unittest.cc, which is just a bunch of
static_asserts for base/template_util.h. The static_asserts here should only
fail if Apple changes their constant scheme again, or retroactively changes the
existing constants, which seems unlikely enough to not care about catching in a
normal build.

 #endif
 
-#if defined(MAC_OS_X_VERSION_10_11) && \
-    MAC_OS_X_VERSION_MIN_REQUIRED > MAC_OS_X_VERSION_10_11
-#define BASE_MAC_MAC_UTIL_H_INLINED_GT_10_11
-inline bool IsOSElCapitan() { return false; }
+#if defined(MAC_OS_X_VERSION_10_12)
+DECLARE_IS_OS_FUNCS(12, MAC_OS_X_VERSION_10_12)
+#else
+DECLARE_IS_OS_FUNCS(12, 0)
 #endif
 
-#if defined(MAC_OS_X_VERSION_10_12) && \
-    MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-#define BASE_MAC_MAC_UTIL_H_INLINED_GE_10_12
-inline bool IsOSSierraOrLater() { return true; }
-#endif
+#undef DECLARE_IS_OS_FUNCS
 
-#if defined(MAC_OS_X_VERSION_10_12) && \
-    MAC_OS_X_VERSION_MIN_REQUIRED > MAC_OS_X_VERSION_10_12
-#define BASE_MAC_MAC_UTIL_H_INLINED_GT_10_12
-inline bool IsOSSierra() { return false; }
-inline bool IsOSLaterThanSierra_DontCallThis() { return true; }
-#endif
+// This should be infrequently used. It only makes sense to use this to avoid
+// codepaths that are very likely to break on future (unreleased, untested,
+// unborn) OS releases, or to log when the OS is newer than any known version.
+inline bool IsOSLaterThan10_12_DontCallThis() {
+  return !IsAtMostOS10_12();
+}
 
 // Retrieve the system's model identifier string from the IOKit registry:
 // for example, "MacPro4,1", "MacBookPro6,1". Returns empty string upon