[cfe-commits] r138993 - /cfe/trunk/lib/Sema/SemaExpr.cpp

[Sebastian Redl]

On 02.09.2011 04:15, Richard Trieu wrote:
> Author: rtrieu
> Date: Thu Sep  1 21:15:37 2011
> New Revision: 138993
>
> URL: http://llvm.org/viewvc/llvm-project?rev=138993&view=rev
> Log:
> Pull out incomplete pointer type checking code, used from arithmetic checking functions, into its own function.
>
> Modified:
>      cfe/trunk/lib/Sema/SemaExpr.cpp
>
> @@ -5815,16 +5833,7 @@
>       return !S.getLangOptions().CPlusPlus;
>     }
>
> -  if ((Operand->getType()->isPointerType()&&
> -       !Operand->getType()->isDependentType()) ||
> -      Operand->getType()->isObjCObjectPointerType()) {
> -    QualType PointeeTy = Operand->getType()->getPointeeType();
> -    if (S.RequireCompleteType(
> -          Loc, PointeeTy,
> -          S.PDiag(diag::err_typecheck_arithmetic_incomplete_type)
> -<<  PointeeTy<<  Operand->getSourceRange()))
> -      return false;
> -  }
> +  if (checkArithmeticIncompletePointerType(S, Loc, Operand)) return false;
>
>     return true;
>   }
Why is the return value convention of caller and callee different? If it 
makes logical sense to make them the same, this could be a simple return 
statement.

> @@ -5869,20 +5878,9 @@
>       return !S.getLangOptions().CPlusPlus;
>     }
>
> -  Expr *Operands[] = { LHS, RHS };
> -  for (unsigned i = 0; i<  2; ++i) {
> -    Expr *Operand = Operands[i];
> -    if ((Operand->getType()->isPointerType()&&
> -         !Operand->getType()->isDependentType()) ||
> -        Operand->getType()->isObjCObjectPointerType()) {
> -      QualType PointeeTy = Operand->getType()->getPointeeType();
> -      if (S.RequireCompleteType(
> -            Loc, PointeeTy,
> -            S.PDiag(diag::err_typecheck_arithmetic_incomplete_type)
> -<<  PointeeTy<<  Operand->getSourceRange()))
> -        return false;
> -    }
> -  }
> +  if (checkArithmeticIncompletePointerType(S, Loc, LHS)) return false;
> +  if (checkArithmeticIncompletePointerType(S, Loc, RHS)) return false;
These look dangerously similar. I had to look thrice to see the 
difference. Renaming LHS and RHS to Left and Right would help. I know 
that LHS and RHS are very commonly used, but this makes me think that's 
a mistake.

Sebastian