[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