[DarwinDriver] Increase the number of valid digits for ld version string

[bruno]

Summary

Previously only 3 digits were valid. Increase it to 5.

[bruno]

Ping!

[bruno]

Ping :-)

[rnk]

lib/Driver/Driver.cpp

2541

You don't need a MaxDigits parameter, it's the same as Digits.size().

[bruno]

Thanks Reid. The original idea was to have MaxDigits despite the size of the input array (but bounded by its size). But since there are no other users up to this point, your suggestion seems better, updated the patch to reflect it.

[dexonsmith]

LGTM from the Darwin-side, assuming Reid doesn't have any more comments.

[rnk]

lgtm, go for it

lib/Driver/Driver.cpp

2549–2550

nit: Why not CurDigit < Digits.size()?

[bruno]

Committed r264987!

lib/Driver/Driver.cpp

2549–2550

No specific reason. Done!

include/clang/Driver/Driver.h

   static bool GetReleaseVersion(const char *Str, unsigned &Major,
                                 unsigned &Minor, unsigned &Micro,
                                 bool &HadExtra);
+
+  /// Parse digits from a string \p Str and fulfill \p Digits with
+  /// the parsed numbers. This method assumes that the max number of
+  /// digits to look for is equal to Digits.size().
+  ///
+  /// \return True if the entire string was parsed and there are
+  /// no extra characters remaining at the end.
+  static bool GetReleaseVersion(const char *Str,
+                                MutableArrayRef<unsigned> Digits);
 };
 
 /// \return True if the last defined optimization level is -Ofast.

lib/Driver/Driver.cpp

   return true;
 }
 
+/// Parse digits from a string \p Str and fulfill \p Digits with
+/// the parsed numbers. This method assumes that the max number of
+/// digits to look for is equal to Digits.size().
+///
+/// \return True if the entire string was parsed and there are
+/// no extra characters remaining at the end.
+bool Driver::GetReleaseVersion(const char *Str,
+                               MutableArrayRef<unsigned> Digits) {
+  if (*Str == '\0')
+    return false;
+
+  char *End;
+  unsigned CurDigit = 0;
+  unsigned MaxDigits = Digits.size();
+  while (CurDigit < MaxDigits) {
+    unsigned Digit = (unsigned)strtol(Str, &End, 10);
+    Digits[CurDigit] = Digit;
+    if (*Str != '\0' && *End == '\0')
+      return true;
+    if (*End != '.' || Str == End)
+      return false;
+    Str = End + 1;
+    CurDigit++;
+  }
+
+  // More digits than requested, bail out...
+  return false;
+}
+
 std::pair<unsigned, unsigned> Driver::getIncludeExcludeOptionFlagMasks() const {
   unsigned IncludedFlagsBitmask = 0;
   unsigned ExcludedFlagsBitmask = options::NoDriverOption;

lib/Driver/Tools.cpp

   const Driver &D = getToolChain().getDriver();
   const toolchains::MachO &MachOTC = getMachOToolChain();
 
-  unsigned Version[3] = {0, 0, 0};
+  unsigned Version[5] = {0, 0, 0, 0, 0};
   if (Arg *A = Args.getLastArg(options::OPT_mlinker_version_EQ)) {
-    bool HadExtra;
-    if (!Driver::GetReleaseVersion(A->getValue(), Version[0], Version[1],
-                                   Version[2], HadExtra) ||
-        HadExtra)
+    if (!Driver::GetReleaseVersion(A->getValue(), Version))
       D.Diag(diag::err_drv_invalid_version_number) << A->getAsString(Args);
   }
 

test/Driver/darwin-ld.c

 // RUN:   FileCheck --check-prefix=LINK-IFRAMEWORK %s
 // LINK-IFRAMEWORK: {{ld(.exe)?"}}
 // LINK-IFRAMEWORK: "-FBar"
+
+// Check ld64 accepts up to 5 digits with no extra characters
+// RUN: %clang -target x86_64-apple-darwin12 %s -### -o %t \
+// RUN:   -mlinker-version=133.3 2> %t.log
+// RUN: %clang -target x86_64-apple-darwin12 %s -### -o %t \
+// RUN:   -mlinker-version=133.3.0 2>> %t.log
+// RUN: %clang -target x86_64-apple-darwin12 %s -### -o %t \
+// RUN:   -mlinker-version=133.3.0.1 2>> %t.log
+// RUN: %clang -target x86_64-apple-darwin12 %s -### -o %t \
+// RUN:   -mlinker-version=133.3.0.1.2 2>> %t.log
+// RUN: %clang -target x86_64-apple-darwin12 %s -### -o %t \
+// RUN:   -mlinker-version=133.3.0.1.2.6 2>> %t.log
+// RUN: %clang -target x86_64-apple-darwin12 %s -### -o %t \
+// RUN:   -mlinker-version=133.3.0.1.a 2>> %t.log
+// RUN: %clang -target x86_64-apple-darwin12 %s -### -o %t \
+// RUN:   -mlinker-version=133.3.0.1a 2>> %t.log
+// RUN: FileCheck -check-prefix=LINK_VERSION_DIGITS %s < %t.log
+// LINK_VERSION_DIGITS-NOT: invalid version number in '-mlinker-version=133.3'
+// LINK_VERSION_DIGITS-NOT: invalid version number in '-mlinker-version=133.3.0'
+// LINK_VERSION_DIGITS-NOT: invalid version number in '-mlinker-version=133.3.0.1'
+// LINK_VERSION_DIGITS-NOT: invalid version number in '-mlinker-version=133.3.0.1.2'
+// LINK_VERSION_DIGITS: invalid version number in '-mlinker-version=133.3.0.1.2.6'
+// LINK_VERSION_DIGITS: invalid version number in '-mlinker-version=133.3.0.1.a'
+// LINK_VERSION_DIGITS: invalid version number in '-mlinker-version=133.3.0.1a'