[PATCH] Add PIC-level support to Clang.
Duncan P. N. Exon Smith
dexonsmith at apple.com
Mon Nov 17 17:50:53 PST 2014
> On 2014-Nov-17, at 15:23, Justin Hibbits <jrh29 at alumni.cwru.edu> wrote:
>
> Thanks for the review, Rafael!
>
> ================
> Comment at: lib/CodeGen/CodeGenModule.cpp:395
> @@ +394,3 @@
> + uint32_t PLevel = Context.getLangOpts().PICLevel;
> + if (PLevel) {
> + llvm::PICLevel::Level PL = llvm::PICLevel::Default;
> ----------------
> rafael wrote:
>> You can fold the variable declaration into the if:
>>
>> if (uint32_t PLevel = Context.getLangOpts().PICLevel) {
> That may be legal C++, but it makes me really uncomfortable, and makes it (trivially, I guess) less readable, I think.
FWIW, it's standard LLVM style to aggressively reduce the scope of
local variables like this. IMO it's actually clearer, since it
limits how far you need to look for references to `PLevel`.
More information about the llvm-commits
mailing list