[PATCH] Add Multilib selection machinery to Clang

Jon Roelofs jonathan at codesourcery.com
Mon Feb 3 16:02:22 PST 2014



================
Comment at: include/clang/Driver/Multilib.h:42
@@ +41,3 @@
+    assert(GCCSuffix.empty() ||
+           (GCCSuffix.front() == '/' && GCCSuffix.size() > 1));
+    return GCCSuffix;
----------------
Simon Atanasyan wrote:
> g++ shows an error on this line. It does not know about std::string::front() method if you do not specify -std=c++11 flag. Have you built the clang by g++ and using autotools configure script or CMake with default options?
I've been using the configure script + make on a mac, where it uses Apple clang to build everything. Looks like the default flags are set slightly differently. I'll fix these and make sure it also builds on a linux box with g++.

================
Comment at: lib/Driver/Multilib.cpp:335
@@ +334,3 @@
+  FilterWrapper(const MultilibSet::FilterCallback &F) : F(F) {}
+  bool operator()(const Multilib &M) const LLVM_OVERRIDE { return F(M); }
+};
----------------
Simon Atanasyan wrote:
> Why do you need `LLVM_OVERRIDE` here?
Oh, you're right. This function isn't virtual. I'll remove it.

================
Comment at: lib/Driver/Multilib.cpp:73
@@ +72,3 @@
+void Multilib::print(raw_ostream &OS) const {
+  assert(GCCSuffix.empty() xor (GCCSuffix.front() == '/'));
+  if (GCCSuffix.empty())
----------------
Same thing about std::string::front() applies here.

Also, this should be '||', not 'xor', otherwise it could crash instead of failing the assert.

================
Comment at: lib/Driver/Multilib.cpp:82
@@ +81,3 @@
+       ++I) {
+    if (I->front() == '+')
+      OS << "@" << I->substr(1);
----------------
Same thing about std::string::front() applies here too.


http://llvm-reviews.chandlerc.com/D2538



More information about the cfe-commits mailing list