[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