<div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 7, 2012 at 11:48 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br>
>> On Tue, Aug 7, 2012 at 10:30 AM, Sam Panzer <<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>> wrote:<br>
>> > Several places in the codebase determine if a method is const or<br>
>> > volatile by<br>
>> > querying the method's type qualifiers directly,<br>
<br>
</div>Oh, sorry I meant to mention this in my previous email: could you<br>
point to the locations that are already doing this manually? and<br>
possibly even update them to use this new utility function so we can<br>
see what kind of cleanup it provides?<br>
<div><div><br></div></div></blockquote><div><br></div><div>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.</div>
<div><br></div><div>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. </div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div>
>> > but it makes sense to<br>
>> > centralize the (simple) logic directly in CXXMethodDecl. It's<br>
>> > particularly<br>
>> > useful for refactoring tools :)<br>
>><br>
>> Given that they're already members on CXXMethodDecl, the "Method" in<br>
>> the name seems a bit redundant (I'm not dead set on this, though - I<br>
>> appreciate that "isConst" might be a bit more unclear than the<br>
>> existing things like "isVirtual", etc) would "isConst" and<br>
>> "isVolatile" suffice?<br>
><br>
><br>
> I considered "isConst" and "isConstQualified" along with "isConstMethod" -<br>
> mostly to distinguish between type qualifier const and method qualifier<br>
> const. I don't think "isConst" is unclear so long as it is accessed from a<br>
> well-named variable.<br>
><br>
>><br>
>><br>
>> Also how do these methods fail if the method is a static member<br>
>> function? Would it be helpful to have an assert there to ensure these<br>
>> are only called on non-static member functions?<br>
><br>
><br>
> Yes! Asserts added.<br>
><br>
</div></div></blockquote></div><br></div>