[PATCH] Add PIC-level support to Clang.

Justin Hibbits jrh29 at alumni.cwru.edu
Mon Nov 17 18:30:10 PST 2014


On Mon, 17 Nov 2014 17:50:53 -0800
"Duncan P. N. Exon Smith" <dexonsmith at apple.com> wrote:

> 
> > 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`.

I can certainly yield to that.



More information about the llvm-commits mailing list