[cfe-commits] [PATCH] cv-qualifiers for C++ methods

[Argiris Kirtzidis]

Douglas Gregor wrote:
>
> On Oct 23, 2008, at 7:21 PM, Argiris Kirtzidis wrote:
>
>> The attached patch implements support for cv-qualifiers in the 
>> function declarator ("void f() const;").
>
>
> (in class FunctionType):
> +  /// TypeQuals - Used only by FunctionTypeProto, put here to pack 
> with the
> +  /// other bitfields.
> +  unsigned TypeQuals : 3;
>
> This is an interesting choice, and I think it's the right one. My 
> first reaction was surprise that the type qualifiers weren't just 
> handled by a QualType, but that would be subtly wrong; the distinction 
> is that there are no cv-qualified function types, but there are 
> function types that have cv qualifiers. This different is probably 
> comment-worthy :)

Yep, and it's actually stated in the standard too, I added the quote.

>
> +    /// The type qualifiers: const/volatile/restrict.
> +    unsigned TypeQuals : 3;
>
> Please note that the qualifier bitmask values are the same as in 
> QualType.

Ok.

>
> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp    (revision 58037)
> +++ lib/Sema/SemaExpr.cpp    (working copy)
> @@ -402,7 +402,8 @@
>        if (FD->isInvalidDecl())
>          return true;
>
> -      return new DeclRefExpr(FD, FD->getType(), Loc);
> +      return new DeclRefExpr(FD,
> +        
> FD->getType().getWithAdditionalQualifiers(MD->getTypeQualifiers()),Loc);
>      }
>
> I wouldn't have expected this, given that the type qualifiers for the 
> function are within the FunctionType. Why are they transferred into 
> the QualType at this point?

FD is a CXXFieldDecl. When a field is referenced like this:

class C {
  int x;
  void m() const { x = 0; } #1
};

at #1, 'x' is represented as a DeclRefExpr with const type.

I added a FIXME about handling 'mutable'.

>
> Should CXXMethodDecl::getThisType be updated to apply the function 
> qualifiers to the type of *this?

Yes, thanks for noticing!

>
> Other than those little things above, I think this patch is ready to 
> go in. Thanks!

Thanks for reviewing!
Commited:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20081020/008537.html

-Argiris