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

[Chandler Carruth]

Just nit-picking a bit...

On Thu, Sep 1, 2011 at 7:15 PM, Richard Trieu <rtrieu at google.com> wrote:

> +/// \brief Warn if Operand is incomplete pointer type
> +///
> +/// \returns True if pointer has incomplete type
> +static bool checkArithmeticIncompletePointerType(Sema &S, SourceLocation
> Loc,
> +                                                 Expr *Operand) {
> +  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()))
>

This if makes my eyes hurt. Maybe put the partial diagnostic in a variable,
and pass that in? Would likely make the whole thing much easier to read.


> +      return true;
> +  }
> +  return false;
> +}
> +
>  /// \brief Check the validity of an arithmetic pointer operand.
>  ///
>  /// If the operand has pointer type, this code will check for pointer
> types
> @@ -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;
>

Here and below, I'd probably put the 'return false;' on its own line so its
easier to spot when scanning.


>
>   return true;
>  }
> @@ -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;
> +
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110901/a91b52a4/attachment.html>