[cfe-commits] [PATCH] Add isConstMethod(), isVolatileMethod() to CXXMethodDecl

[Sam Panzer]

On Tue, Aug 7, 2012 at 11:48 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
> >> On Tue, Aug 7, 2012 at 10:30 AM, Sam Panzer <panzer at google.com> wrote:
> >> > Several places in the codebase determine if a method is const or
> >> > volatile by
> >> > querying the method's type qualifiers directly,
>
> Oh, sorry I meant to mention this in my previous email: could you
> point to the locations that are already doing this manually? and
> possibly even update them to use this new utility function so we can
> see what kind of cleanup it provides?
>
>
That would make sense. I found a number of uses that I could fix directly,
though some of them blindly check to see if a function's type is const
qualified without checking to see if it's actually a member function. I
moved isConst, isVolatile, and isRestrict into FunctionType and exposed
them from CXXMethodDecl, after removing the assert again. One potential
downside is that MethodDecl.getType().isConstQualified() will still not do
what I originally expected.

The new patch removes as many usages of directly checking Qualifiers bits
as I thought I could change and removes one of the (at least) two different
ways to do this check (i.e.
Qualifiers::fromCVRMask(Method->getTypeQualifiers()).hasConst()) entirely.



>  >> > but it makes sense to
> >> > centralize the (simple) logic directly in CXXMethodDecl. It's
> >> > particularly
> >> > useful for refactoring tools :)
> >>
> >> Given that they're already members on CXXMethodDecl, the "Method" in
> >> the name seems a bit redundant (I'm not dead set on this, though - I
> >> appreciate that "isConst" might be a bit more unclear than the
> >> existing things like "isVirtual", etc) would "isConst" and
> >> "isVolatile" suffice?
> >
> >
> > I considered "isConst" and "isConstQualified" along with "isConstMethod"
> -
> > mostly to distinguish between type qualifier const and method qualifier
> > const. I don't think "isConst" is unclear so long as it is accessed from
> a
> > well-named variable.
> >
> >>
> >>
> >> Also how do these methods fail if the method is a static member
> >> function? Would it be helpful to have an assert there to ensure these
> >> are only called on non-static member functions?
> >
> >
> > Yes! Asserts added.
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120808/ccc776b5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: isConst.patch
Type: application/octet-stream
Size: 6638 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120808/ccc776b5/attachment.obj>