This distinguishes between -fpic and -fPIC now, with the additions in LLVM for
PIC level support.
Details
Diff Detail
Event Timeline
lib/CodeGen/BackendUtil.cpp | ||
---|---|---|
428 ↗ | (On Diff #14425) | Don't you want to produce an error if -pic-level is not 1 or 2? |
lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
401 | We use llvm_unreachble instead of assert(0). |
lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
401 | d'oh, fail :( |
LGTM with a nit.
lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
395 | You can fold the variable declaration into the if: if (uint32_t PLevel = Context.getLangOpts().PICLevel) { |
lib/CodeGen/BackendUtil.cpp | ||
---|---|---|
428 ↗ | (On Diff #14425) | It's too easy for the user to try to do something like clang -cc1 -pic-level 3 t.c and crash the compiler with assert or llvm_unreachable. We should either clamp the level to [0-2] or issue a diagnostic in lib/Frontend/CompilerInvocation.cpp. |
Thanks for the review, Rafael!
lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
395 | That may be legal C++, but it makes me really uncomfortable, and makes it (trivially, I guess) less readable, I think. |
rafael wrote:
Don't you want to produce an error if -pic-level is not 1 or 2?
It's too easy for the user to try to do something like clang -cc1 -pic-level 3 t.c and crash the compiler with assert or llvm_unreachable. We should either clamp the level to [0-2] or issue a diagnostic in lib/Frontend/CompilerInvocation.cpp.
I don't think -cc1 options are considered user visible. Having another
value in here means a bug somewhere else in clang.
Cheers,
Rafael
You can fold the variable declaration into the if:
if (uint32_t PLevel = Context.getLangOpts().PICLevel) {