[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