[llvm] r245712 - Remove an unnecessary use of pointee types introduced in r194220

[David Blaikie via llvm-commits]

On Fri, Aug 21, 2015 at 1:22 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2015-Aug-21, at 13:19, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Fri, Aug 21, 2015 at 12:16 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >
> > > On 2015-Aug-21, at 10:37, David Blaikie via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> > >
> > > Author: dblaikie
> > > Date: Fri Aug 21 12:37:41 2015
> > > New Revision: 245712
> > >
> > > URL:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D245712-26view-3Drev&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=vftFLnHiqThJHdL0qZWd_Vo12qdMVOZDFnNVBhP9GKA&m=iFuuRajlg8CK1MZggc-FONg4WvEb9dEMjTcf2snpqjE&s=wcuxTBHNGsUIr6klIeRbCLTEYihMxpwXPg8mbp6uLCY&e=
> > > Log:
> > > Remove an unnecessary use of pointee types introduced in r194220
> > >
> > > David Majnemer (the original author) believes this to be an impossible
> > > condition to reach anyway, and no test cases cover this so we'll go
> with
> > > that.
> > >
> > > Modified:
> > >    llvm/trunk/lib/IR/ConstantFold.cpp
> > >
> > > Modified: llvm/trunk/lib/IR/ConstantFold.cpp
> > > URL:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_IR_ConstantFold.cpp-3Frev-3D245712-26r1-3D245711-26r2-3D245712-26view-3Ddiff&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=vftFLnHiqThJHdL0qZWd_Vo12qdMVOZDFnNVBhP9GKA&m=iFuuRajlg8CK1MZggc-FONg4WvEb9dEMjTcf2snpqjE&s=9qg0E1KghzGtl2ckpYoi8CzXjWBQthtOX6aXtRz5Jh8&e=
> > >
> ==============================================================================
> > > --- llvm/trunk/lib/IR/ConstantFold.cpp (original)
> > > +++ llvm/trunk/lib/IR/ConstantFold.cpp Fri Aug 21 12:37:41 2015
> > > @@ -1999,9 +1999,8 @@ static bool isInBoundsIndices(ArrayRef<I
> > > /// \brief Test whether a given ConstantInt is in-range for a
> SequentialType.
> > > static bool isIndexInRangeOfSequentialType(SequentialType *STy,
> > >                                            const ConstantInt *CI) {
> > > -  if (auto *PTy = dyn_cast<PointerType>(STy))
> > > -    // Only handle pointers to sized types, not pointers to functions.
> > > -    return PTy->getElementType()->isSized();
> > > +  if (isa<PointerType>(STy))
> > > +    return true;
> >
> > Worth a comment in the code?
> >
> > Not sure if a comment would add value - the existing comment was just to
> describe a special case that didn't really exist/couldn't come up, so it
> seemed like without that the code would be fine as-is.
> >
> > But sure, added in r245730
>
> If you don't think it adds value, then feel free to revert;


I think it adds a bit - I tend to be a bit... terse, when it comes to code,
so I usually appreciate the prodding to be more explanatory.


> I was
> just wondering if someone reading the code would ask the same
> question you did originally (and worry that a case was missing).
>

I don't /think/ so, but I could be wrong. I've commented the intent without
calling out teh special case, which I don't /think/ would occur to anyone
looking at the code, but I could be wrong.


>
> >
> >    // And indicies are valid when indexing along a pointer
> >     if (isa<PointerType>(STy))
> >       return true;
> >
> >
> >
> > >
> > >   uint64_t NumElements = 0;
> > >   // Determine the number of elements in our sequential type.
> > >
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at lists.llvm.org
> > >
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=BQIGaQ&c=eEvniauFctOgLOKGJOplqw&r=vftFLnHiqThJHdL0qZWd_Vo12qdMVOZDFnNVBhP9GKA&m=iFuuRajlg8CK1MZggc-FONg4WvEb9dEMjTcf2snpqjE&s=CrF61doRZdRF05PkR5nctXedsIRfGq3hcZrivnRnW1A&e=
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150821/5a3dcad2/attachment.html>