================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
