[Clang] Add clangMinimumVersion to multilib.yaml

[michaelplatings]

Summary

This avoids a potential source of subtle errors if an older version of
Clang were to be used with a multilib.yaml that requires a newer Clang
to work correctly.

This feature is comparable to CMake's cmake_minimum_required.

[MaskRay]

I think this patch can be merged into a previous one that introduces variants and flagMap

[peter.smith]

No objections to MaskRay's suggestion to merge with an earlier patch. I've made some suggestions to make the error messages be a bit more specific about what is wrong.

clang/lib/Driver/Multilib.cpp

185

Perhaps
"missing required key 'clangMinimumVersion' expect MAJOR.MINOR.PATCHLEVEL" to give an idea what the key is expecting?

195

Can we add why the version string isn't valid? For example "expected format MAJOR.MINOR.PATCHLEVEL got " + M.CLangMinimumVersion ...

200

Similarly, can we say what we were expecting. For example all components must be decimal integers.

[michaelplatings]

Squashed into D142932

clang/lib/Driver/Multilib.cpp

 //===- Multilib.cpp - Multilib Implementation -----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Driver/Multilib.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"

 }
 
 struct MultilibSerialization {
   std::string Dir;
   std::vector<std::string> Flags, PrintArgs;
 };
 
 struct MultilibSetSerialization {
+  std::string ClangMinimumVersion;
   std::vector<MultilibSerialization> Multilibs;
   std::vector<MultilibSet::FlagMatcher> FlagMatchers;
 };
 
 template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
   static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
     io.mapRequired("dir", V.Dir);
     io.mapRequired("flags", V.Flags);
static std::string validate(IO &io, MultilibSet::FlagMatcher &M) {
     if (M.MatchFlags.empty() && M.NoMatchFlags.empty())
       return "value required for 'matchFlags' or 'noMatchFlags'";
     return std::string{};
   }
 };
 
 template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> {
   static void mapping(llvm::yaml::IO &io, MultilibSetSerialization &M) {
+    io.mapRequired("clangMinimumVersion", M.ClangMinimumVersion);
     io.mapRequired("variants", M.Multilibs);
     io.mapOptional("flagMap", M.FlagMatchers);
   }
+  static std::string validate(IO &io, MultilibSetSerialization &M) {
+    if (M.ClangMinimumVersion.empty())
+      return "missing required key 'clangMinimumVersion'";
+
+    SmallVector<int, 3> ClangMinimumVersion,
+        ClangVersion = {CLANG_VERSION_MAJOR, CLANG_VERSION_MINOR,
+                        CLANG_VERSION_PATCHLEVEL};
+
+    SmallVector<StringRef, 3> MinVerStrings;
+    StringRef(M.ClangMinimumVersion).split(MinVerStrings, '.');
+
+    if (MinVerStrings.size() != 3)
+      return "not a valid version string \"" + M.ClangMinimumVersion + "\"";
+
+    for (StringRef S : MinVerStrings) {
+      int V;
+      if (S.getAsInteger(10, V))
+        return "not a valid version string \"" + M.ClangMinimumVersion + "\"";
+      ClangMinimumVersion.push_back(V);
+    }
+
+    if (ClangVersion < ClangMinimumVersion) {
+      return "clang version " CLANG_VERSION_STRING
+             " is less than clangMinimumVersion: " +
+             M.ClangMinimumVersion;
+    }
+    return std::string{};
+  }
 };
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSerialization)
 LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSet::FlagMatcher)
 
 bool MultilibSet::parse(llvm::MemoryBufferRef Input,
                         llvm::SourceMgr::DiagHandlerTy DiagHandler,
                         void *DiagHandlerCtxt) {

clang/unittests/Driver/MultilibTest.cpp

 //===- unittests/Driver/MultilibTest.cpp --- Multilib tests ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Unit tests for Multilib and MultilibSet
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Driver/Multilib.h"
 #include "../../lib/Driver/ToolChains/CommonArgs.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 
 using namespace clang::driver;
 using namespace clang;
static void diagnosticCallback(const llvm::SMDiagnostic &D, void *Out) {
   *reinterpret_cast<std::string *>(Out) = D.getMessage();
 }
 
 static bool parse(MultilibSet &MS, std::string &Diagnostic, const char *Data) {
   return MS.parse(llvm::MemoryBufferRef(Data, "TEST"), diagnosticCallback,
                   &Diagnostic);
 }
 
+static bool parse(MultilibSet &MS, std::string &Diagnostic,
+                  const std::string &Data) {
+  return MS.parse(llvm::MemoryBufferRef(Data, "TEST"), diagnosticCallback,
+                  &Diagnostic);
+}
+
 static bool parse(MultilibSet &MS, const char *Data) {
   return MS.parse(llvm::MemoryBufferRef(Data, "TEST"));
 }
 
+#define _STRINGIFY(x) #x
+#define STRINGIFY(x) _STRINGIFY(x)
+// Avoid using MULTILIB_CLANG_VERSION in case it has extra non-numeric parts.
+#define MULTILIB_CLANG_VERSION                                                 \
+  STRINGIFY(CLANG_VERSION_MAJOR)                                               \
+  "." STRINGIFY(CLANG_VERSION_MINOR) "." STRINGIFY(CLANG_VERSION_PATCHLEVEL)
+#define YAML_PREAMBLE "clangMinimumVersion: " MULTILIB_CLANG_VERSION "\n"
+
 TEST(MultilibTest, ParseInvalid) {
   std::string Diagnostic;
 
   MultilibSet MS;
 
-  EXPECT_FALSE(parse(MS, Diagnostic, "{}"));
-  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'variants'"))
+  EXPECT_FALSE(parse(MS, Diagnostic, R"(
+variants: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic)
+                  .contains("missing required key 'clangMinimumVersion'"))
       << Diagnostic;
 
+  // Require all 3 major.minor.patch version components
   EXPECT_FALSE(parse(MS, Diagnostic, R"(
+clangMinimumVersion: )" STRINGIFY(CLANG_VERSION_MAJOR) R"(.0
+variants: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic)
+                  .contains("not a valid version string \"" STRINGIFY(
+                      CLANG_VERSION_MAJOR) ".0\""))
+      << Diagnostic;
+
+  EXPECT_FALSE(parse(MS, Diagnostic, R"(
+clangMinimumVersion: )" MULTILIB_CLANG_VERSION R"(a
+variants: []
+)"));
+  EXPECT_TRUE(
+      StringRef(Diagnostic)
+          .contains("not a valid version string \"" MULTILIB_CLANG_VERSION
+                    "a\""))
+      << Diagnostic;
+
+  // Reject configurations that require a later clang version
+  EXPECT_FALSE(parse(MS, Diagnostic,
+                     R"(
+clangMinimumVersion: )" + std::to_string(CLANG_VERSION_MAJOR + 1) +
+                         R"(.0.0
+variants: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic)
+                  .contains("clang version " MULTILIB_CLANG_VERSION
+                            " is less than clangMinimumVersion: " +
+                            std::to_string(CLANG_VERSION_MAJOR + 1) + ".0.0"))
+      << Diagnostic;
+
+  // but accept configurations that only need an earlier clang version
+  EXPECT_TRUE(parse(MS, Diagnostic, R"(
+clangMinimumVersion: 16.0.0
+variants: []
+)")) << Diagnostic;
+
+  EXPECT_FALSE(parse(MS, Diagnostic, YAML_PREAMBLE));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'variants'"))
+      << Diagnostic;
+
+  EXPECT_FALSE(parse(MS, Diagnostic, YAML_PREAMBLE R"(
 variants:
 - dir: /abc
   flags: []
   printArgs: []
 )"));
   EXPECT_TRUE(StringRef(Diagnostic).contains("paths must be relative"))
       << Diagnostic;
 
-  EXPECT_FALSE(parse(MS, Diagnostic, R"(
+  EXPECT_FALSE(parse(MS, Diagnostic, YAML_PREAMBLE R"(
 variants:
 - flags: []
   printArgs: []
 )"));
   EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'dir'"))
       << Diagnostic;
 
-  EXPECT_FALSE(parse(MS, Diagnostic, R"(
+  EXPECT_FALSE(parse(MS, Diagnostic, YAML_PREAMBLE R"(
 variants:
 - dir: .
   printArgs: []
 )"));
   EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'flags'"))
       << Diagnostic;
 
-  EXPECT_FALSE(parse(MS, Diagnostic, R"(
+  EXPECT_FALSE(parse(MS, Diagnostic, YAML_PREAMBLE R"(
 variants:
 - dir: .
   flags: []
 )"));
   EXPECT_TRUE(
       StringRef(Diagnostic).contains("missing required key 'printArgs'"))
       << Diagnostic;
 
-  EXPECT_FALSE(parse(MS, Diagnostic, R"(
+  EXPECT_FALSE(parse(MS, Diagnostic, YAML_PREAMBLE R"(
 variants: []
 flagMap:
 - regex: abc
 )"));
   EXPECT_TRUE(
       StringRef(Diagnostic)
           .contains("value required for 'matchFlags' or 'noMatchFlags'"))
       << Diagnostic;
 
-  EXPECT_FALSE(parse(MS, Diagnostic, R"(
+  EXPECT_FALSE(parse(MS, Diagnostic, YAML_PREAMBLE R"(
 variants: []
 flagMap:
 - dir: .
   regex: '('
   printArgs: []
 
 )"));
   EXPECT_TRUE(StringRef(Diagnostic).contains("parentheses not balanced"))
       << Diagnostic;
 }
 
 TEST(MultilibTest, Parse) {
   MultilibSet MS;
-  EXPECT_TRUE(parse(MS, R"(
+  EXPECT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: .
   flags: []
   printArgs: []
 )"));
   EXPECT_EQ(1U, MS.size());
   EXPECT_EQ("", MS.begin()->gccSuffix());
 
-  EXPECT_TRUE(parse(MS, R"(
+  EXPECT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: abc
   flags: []
   printArgs: []
 )"));
   EXPECT_EQ(1U, MS.size());
   EXPECT_EQ("/abc", MS.begin()->gccSuffix());
 
-  EXPECT_TRUE(parse(MS, R"(
+  EXPECT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: pqr
   flags: []
   printArgs: [-mfloat-abi=soft]
 )"));
   EXPECT_EQ(1U, MS.size());
   EXPECT_EQ("/pqr", MS.begin()->gccSuffix());
   EXPECT_EQ(std::vector<std::string>({"-mfloat-abi=soft"}),
             MS.begin()->getPrintArgs());
 
-  EXPECT_TRUE(parse(MS, R"(
+  EXPECT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: pqr
   flags: []
   printArgs: [-mfloat-abi=soft, -fno-exceptions]
 )"));
   EXPECT_EQ(1U, MS.size());
   EXPECT_EQ(std::vector<std::string>({"-mfloat-abi=soft", "-fno-exceptions"}),
             MS.begin()->getPrintArgs());
 
-  EXPECT_TRUE(parse(MS, R"(
+  EXPECT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: a
   flags: []
   printArgs: []
 - dir: b
   flags: []
   printArgs: []
 )"));
   EXPECT_EQ(2U, MS.size());
 }
 
 TEST(MultilibTest, SelectSoft) {
   MultilibSet MS;
   Multilib Selected;
-  ASSERT_TRUE(parse(MS, R"(
+  ASSERT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: s
   flags: [softabi]
   printArgs: []
 flagMap:
 - regex: mfloat-abi=soft
   matchFlags: [softabi]
 - regex: mfloat-abi=softfp
   matchFlags: [softabi]
 )"));
   EXPECT_TRUE(MS.select({"mfloat-abi=soft"}, Selected));
   EXPECT_TRUE(MS.select({"mfloat-abi=softfp"}, Selected));
   EXPECT_FALSE(MS.select({"mfloat-abi=hard"}, Selected));
 }
 
 TEST(MultilibTest, SelectSoftFP) {
   MultilibSet MS;
   Multilib Selected;
-  ASSERT_TRUE(parse(MS, R"(
+  ASSERT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: f
   flags: [mfloat-abi=softfp]
   printArgs: []
 )"));
   EXPECT_FALSE(MS.select({"mfloat-abi=soft"}, Selected));
   EXPECT_TRUE(MS.select({"mfloat-abi=softfp"}, Selected));
   EXPECT_FALSE(MS.select({"mfloat-abi=hard"}, Selected));
 }
 
 TEST(MultilibTest, SelectHard) {
   // If hard float is all that's available then select that only if compiling
   // with hard float.
   MultilibSet MS;
   Multilib Selected;
-  ASSERT_TRUE(parse(MS, R"(
+  ASSERT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: h
   flags: [mfloat-abi=hard]
   printArgs: []
 )"));
   EXPECT_FALSE(MS.select({"mfloat-abi=soft"}, Selected));
   EXPECT_FALSE(MS.select({"mfloat-abi=softfp"}, Selected));
   EXPECT_TRUE(MS.select({"mfloat-abi=hard"}, Selected));
 }
 
 TEST(MultilibTest, SelectFloatABI) {
   MultilibSet MS;
   Multilib Selected;
-  ASSERT_TRUE(parse(MS, R"(
+  ASSERT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: s
   flags: [softabi]
   printArgs: []
 - dir: f
   flags: [softabi, hasfp]
   printArgs: []
 - dir: h
)"));
   EXPECT_EQ("/h", Selected.gccSuffix());
 }
 
 TEST(MultilibTest, SelectFloatABIReversed) {
   // If soft is specified after softfp then softfp will never be
   // selected because soft is compatible with softfp and last wins.
   MultilibSet MS;
   Multilib Selected;
-  ASSERT_TRUE(parse(MS, R"(
+  ASSERT_TRUE(parse(MS, YAML_PREAMBLE R"(
 variants:
 - dir: h
   flags: [hardabi, hasfp]
   printArgs: []
 - dir: f
   flags: [softabi, hasfp]
   printArgs: []
 - dir: s
)"));
   EXPECT_EQ("/s", Selected.gccSuffix());
   MS.select({"mfloat-abi=softfp"}, Selected);
   EXPECT_EQ("/s", Selected.gccSuffix());
   MS.select({"mfloat-abi=hard"}, Selected);
   EXPECT_EQ("/h", Selected.gccSuffix());
 }
 
 TEST(MultilibTest, SelectMClass) {
-  const char *MultilibSpec = R"(
+  const char *MultilibSpec = YAML_PREAMBLE R"(
 variants:
 - dir: thumb/v6-m/nofp
   flags: [target=thumbv6m-none-eabi]
   printArgs: [--target=thumbv6m-none-eabi, -mfloat-abi=soft]
 
 - dir: thumb/v7-m/nofp
   flags: [target=thumbv7m-none-eabi]
   printArgs: [--target=thumbv7m-none-eabi, -mfloat-abi=soft]