[PATCH] D143763: [Clang] Add clangMinimumVersion to multilib.yaml

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

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.



================
Comment at: clang/lib/Driver/Multilib.cpp:185
+    if (M.ClangMinimumVersion.empty())
+      return "missing required key 'clangMinimumVersion'";
+
----------------
Perhaps
"missing required key 'clangMinimumVersion' expect MAJOR.MINOR.PATCHLEVEL" to give an idea what the key is expecting?


================
Comment at: clang/lib/Driver/Multilib.cpp:195
+    if (MinVerStrings.size() != 3)
+      return "not a valid version string \"" + M.ClangMinimumVersion + "\"";
+
----------------
Can we add why the version string isn't valid? For example "expected format MAJOR.MINOR.PATCHLEVEL got " + M.CLangMinimumVersion ...




================
Comment at: clang/lib/Driver/Multilib.cpp:200
+      if (S.getAsInteger(10, V))
+        return "not a valid version string \"" + M.ClangMinimumVersion + "\"";
+      ClangMinimumVersion.push_back(V);
----------------
Similarly, can we say what we were expecting. For example all components must be decimal integers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143763