[cfe-commits] r94173 - in /cfe/trunk: lib/Sema/SemaDeclCXX.cpp test/SemaCXX/virtual-override.cpp

[Douglas Gregor]

On Jan 22, 2010, at 5:07 AM, Chandler Carruth wrote:

> Author: chandlerc
> Date: Fri Jan 22 07:07:41 2010
> New Revision: 94173
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=94173&view=rev
> Log:
> Fix an obvious goof that caused us to only see the top level of return types
> when checking for covariance. Added some fun test cases, fixes PR6110.
> 
> This felt obvious enough to just commit. ;] Let me know if anything needs
> tweaking.
> 
> Modified:
>    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>    cfe/trunk/test/SemaCXX/virtual-override.cpp
> 
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=94173&r1=94172&r2=94173&view=diff
> 
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Jan 22 07:07:41 2010
> @@ -5622,13 +5622,13 @@
>   QualType NewClassTy, OldClassTy;
> 
>   /// Both types must be pointers or references to classes.
> -  if (PointerType *NewPT = dyn_cast<PointerType>(NewTy)) {
> -    if (PointerType *OldPT = dyn_cast<PointerType>(OldTy)) {
> +  if (PointerType *NewPT = dyn_cast<PointerType>(CNewTy)) {
> +    if (PointerType *OldPT = dyn_cast<PointerType>(COldTy)) {
>       NewClassTy = NewPT->getPointeeType();
>       OldClassTy = OldPT->getPointeeType();
>     }
> -  } else if (ReferenceType *NewRT = dyn_cast<ReferenceType>(NewTy)) {
> -    if (ReferenceType *OldRT = dyn_cast<ReferenceType>(OldTy)) {
> +  } else if (ReferenceType *NewRT = dyn_cast<ReferenceType>(CNewTy)) {
> +    if (ReferenceType *OldRT = dyn_cast<ReferenceType>(COldTy)) {
>       NewClassTy = NewRT->getPointeeType();
>       OldClassTy = OldRT->getPointeeType();
>     }

I find this code a bit strange... why is it mapping down to the canonical type then using dyn_cast? It would be simpler just to keep the original source types and use getAs<> rather than dyn_cast. Then we'll get better typedef information in the diagnostics, and I won't have to panic every time I see dyn_cast<> on types.

Alternatively, if we really think we have to work on canonical types, we should use CanQualType or CanQual<>.

	- Doug