[PATCH] Driver: support detecting driver mode when clang has a version suffix without dash (PR21094)

[David Blaikie]

================
Comment at: tools/driver/driver.cpp:209
@@ +208,3 @@
+      {"clang",       nullptr},
+      {"clang++",     "--driver-mode=g++"},
+      {"clang-c++",   "--driver-mode=g++"},
----------------
hans wrote:
> dblaikie wrote:
> > possible cleanup: could remove the duplicated "--driver-mode=" from these strings & just build it into the use of these strings instead.
> I'd rather skip on this, since it will require allocating storage for the string concatenation somewhere, so making this code simpler will make the code that adds the flag to the arguments less straight-forward.
Oh, fair enough - that makes sense. Thanks for the explanation.

================
Comment at: tools/driver/driver.cpp:249
@@ +248,3 @@
+  if (SuffixIndex == -1) {
+    // Try again after stripping any trailing version number.
+    while (!ProgNameRef.empty() &&
----------------
hans wrote:
> dblaikie wrote:
> > this seems like a bit of a special case (& rather repetitious) - could we not just have a single call to FindDriverSuffix search for an arbitrary substring rather than a suffix?
> > 
> > What if someone has clang++-my-branch ? (perhaps not very common, I suppose - but still, a substring search seems simpler and more accepting - would it go wrong somewhere?)
> I'm hesitant to change the existing suffix-matching logic too much. clang++-my-branch won't work, but it didn't work before my patch either.
> 
> Also it wouldn't be completely straight-forward: cl is a substring of clang, so we'd have to be somewhat smart, and with a target prefix mixed in... hmm.
OK then - but at this point I'm a bit too confused by the logic here (test if it's a suffix, if it isn't, strip off any digits and dots (a version suffix), try to find the suffix again, then strip off any -foo component and try again... 

Oh, I see one problem then, maybe... (next comment)

================
Comment at: tools/driver/driver.cpp:250
@@ +249,3 @@
+    // Try again after stripping any trailing version number.
+    while (!ProgNameRef.empty() &&
+           (isDigit(ProgNameRef.back()) || ProgNameRef.back() == '.'))
----------------
Hmm, pity this is essentially std::string's find_last_not_of, except that you can't readily pass the set of all digits and '.'. I could imagine some other STL algorithms that I might find more readable, but probably only about as much as the std::find_if discussed previously.

================
Comment at: tools/driver/driver.cpp:251
@@ +250,3 @@
+    while (!ProgNameRef.empty() &&
+           (isDigit(ProgNameRef.back()) || ProgNameRef.back() == '.'))
+      ProgNameRef = ProgNameRef.drop_back(1);
----------------
the extra parens around the isDigit call are a bit confusing/misleading (I saw the end ')' and thought it was maybe grouped with the empty() call above for some reason)

================
Comment at: tools/driver/driver.cpp:254
@@ +253,3 @@
+  }
+  SuffixIndex = FindDriverSuffix(ProgNameRef);
+
----------------
Should this line be inside the prior if (but after/outside the loop)?

http://reviews.llvm.org/D5833