[cfe-commits] r162468 - in /cfe/trunk/lib/AST: CXXInheritance.cpp ExprConstant.cpp

David Blaikie dblaikie at gmail.com
Mon Aug 27 14:32:50 PDT 2012


On Thu, Aug 23, 2012 at 1:46 PM, Ted Kremenek <kremenek at apple.com> wrote:
> Author: kremenek
> Date: Thu Aug 23 15:46:57 2012
> New Revision: 162468
>
> URL: http://llvm.org/viewvc/llvm-project?rev=162468&view=rev
> Log:
> Change a bunch of cases where we do "getAs<...>->doSomething()" to
> "castAs<...>->doSomething()".  The analyzer was flagging these
> as potential null dereferences, which is technically true.  The
> invariants appear to be that these casts should never fail, so
> let's use castAs<> instead and avoid a runtime check.

Funny you should mention this - Sam Panzer & I came across this issue
(& he went & did a cursory search over the Clang code) a few days ago.

"Just for fun, I ran a grep for getAs() followed immediately by a
method call, which turned up 295 cases that should probably be
castAs(). If this sort of thing should be cleaned up...
$ git grep -E '(->|\.)getAs<\w+>\(\)->' | wc -l"

There are a few less with your change, but still a few to clean up. I
might get to them at some point - but figured I'd record it here
because it seemed relevant to the change. (perhaps they're also
interesting to you as areas of improvement for the Static Analyzer if
it's not correctly flagging them already)

- David

>
> Modified:
>     cfe/trunk/lib/AST/CXXInheritance.cpp
>     cfe/trunk/lib/AST/ExprConstant.cpp
>
> Modified: cfe/trunk/lib/AST/CXXInheritance.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXInheritance.cpp?rev=162468&r1=162467&r2=162468&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/CXXInheritance.cpp (original)
> +++ cfe/trunk/lib/AST/CXXInheritance.cpp Thu Aug 23 15:46:57 2012
> @@ -363,8 +363,8 @@
>                                    void *BaseRecord) {
>    assert(((Decl *)BaseRecord)->getCanonicalDecl() == BaseRecord &&
>           "User data for FindBaseClass is not canonical!");
> -  return Specifier->getType()->getAs<RecordType>()->getDecl()
> -           ->getCanonicalDecl() == BaseRecord;
> +  return Specifier->getType()->castAs<RecordType>()->getDecl()
> +            ->getCanonicalDecl() == BaseRecord;
>  }
>
>  bool CXXRecordDecl::FindVirtualBaseClass(const CXXBaseSpecifier *Specifier,
> @@ -373,14 +373,15 @@
>    assert(((Decl *)BaseRecord)->getCanonicalDecl() == BaseRecord &&
>           "User data for FindBaseClass is not canonical!");
>    return Specifier->isVirtual() &&
> -         Specifier->getType()->getAs<RecordType>()->getDecl()
> -           ->getCanonicalDecl() == BaseRecord;
> +         Specifier->getType()->castAs<RecordType>()->getDecl()
> +            ->getCanonicalDecl() == BaseRecord;
>  }
>
>  bool CXXRecordDecl::FindTagMember(const CXXBaseSpecifier *Specifier,
>                                    CXXBasePath &Path,
>                                    void *Name) {
> -  RecordDecl *BaseRecord = Specifier->getType()->getAs<RecordType>()->getDecl();
> +  RecordDecl *BaseRecord =
> +    Specifier->getType()->castAs<RecordType>()->getDecl();
>
>    DeclarationName N = DeclarationName::getFromOpaquePtr(Name);
>    for (Path.Decls = BaseRecord->lookup(N);
> @@ -396,7 +397,8 @@
>  bool CXXRecordDecl::FindOrdinaryMember(const CXXBaseSpecifier *Specifier,
>                                         CXXBasePath &Path,
>                                         void *Name) {
> -  RecordDecl *BaseRecord = Specifier->getType()->getAs<RecordType>()->getDecl();
> +  RecordDecl *BaseRecord =
> +    Specifier->getType()->castAs<RecordType>()->getDecl();
>
>    const unsigned IDNS = IDNS_Ordinary | IDNS_Tag | IDNS_Member;
>    DeclarationName N = DeclarationName::getFromOpaquePtr(Name);
> @@ -414,7 +416,8 @@
>  FindNestedNameSpecifierMember(const CXXBaseSpecifier *Specifier,
>                                CXXBasePath &Path,
>                                void *Name) {
> -  RecordDecl *BaseRecord = Specifier->getType()->getAs<RecordType>()->getDecl();
> +  RecordDecl *BaseRecord =
> +    Specifier->getType()->castAs<RecordType>()->getDecl();
>
>    DeclarationName N = DeclarationName::getFromOpaquePtr(Name);
>    for (Path.Decls = BaseRecord->lookup(N);
> @@ -692,7 +695,7 @@
>             "Cannot get indirect primary bases for class with dependent bases.");
>
>      const CXXRecordDecl *BaseDecl =
> -      cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
> +      cast<CXXRecordDecl>(I->getType()->castAs<RecordType>()->getDecl());
>
>      // Only bases with virtual bases participate in computing the
>      // indirect primary virtual base classes.
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=162468&r1=162467&r2=162468&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Aug 23 15:46:57 2012
> @@ -953,7 +953,7 @@
>    if (VD)
>      Info.Note(VD->getLocation(), diag::note_declared_at);
>    else
> -    Info.Note(Base.dyn_cast<const Expr*>()->getExprLoc(),
> +    Info.Note(Base.get<const Expr*>()->getExprLoc(),
>                diag::note_constexpr_temporary_here);
>  }
>
> @@ -2586,7 +2586,7 @@
>      const FieldDecl *FD = dyn_cast<FieldDecl>(E->getMemberDecl());
>      if (!FD) return Error(E);
>      assert(!FD->getType()->isReferenceType() && "prvalue reference?");
> -    assert(BaseTy->getAs<RecordType>()->getDecl()->getCanonicalDecl() ==
> +    assert(BaseTy->castAs<RecordType>()->getDecl()->getCanonicalDecl() ==
>             FD->getParent()->getCanonicalDecl() && "record / field mismatch");
>
>      SubobjectDesignator Designator(BaseTy);
> @@ -2665,7 +2665,7 @@
>      if (E->isArrow()) {
>        if (!EvaluatePointer(E->getBase(), Result, this->Info))
>          return false;
> -      BaseTy = E->getBase()->getType()->getAs<PointerType>()->getPointeeType();
> +      BaseTy = E->getBase()->getType()->castAs<PointerType>()->getPointeeType();
>      } else if (E->getBase()->isRValue()) {
>        assert(E->getBase()->getType()->isRecordType());
>        if (!EvaluateTemporary(E->getBase(), Result, this->Info))
> @@ -3036,7 +3036,7 @@
>    if (E->getOpcode() == BO_Sub)
>      AdditionalOffset = -AdditionalOffset;
>
> -  QualType Pointee = PExp->getType()->getAs<PointerType>()->getPointeeType();
> +  QualType Pointee = PExp->getType()->castAs<PointerType>()->getPointeeType();
>    return HandleLValueArrayAdjustment(Info, E, Result, Pointee,
>                                       AdditionalOffset);
>  }
> @@ -5176,7 +5176,7 @@
>      QualType Ty = E->getTypeOfArgument();
>
>      if (Ty->isVectorType()) {
> -      unsigned n = Ty->getAs<VectorType>()->getNumElements();
> +      unsigned n = Ty->castAs<VectorType>()->getNumElements();
>
>        // The vec_step built-in functions that take a 3-component
>        // vector return 4. (OpenCL 1.1 spec 6.11.12)
> @@ -5753,7 +5753,7 @@
>  }
>
>  bool ComplexExprEvaluator::ZeroInitialization(const Expr *E) {
> -  QualType ElemTy = E->getType()->getAs<ComplexType>()->getElementType();
> +  QualType ElemTy = E->getType()->castAs<ComplexType>()->getElementType();
>    if (ElemTy->isRealFloatingType()) {
>      Result.makeComplexFloat();
>      APFloat Zero = APFloat::getZero(Info.Ctx.getFloatTypeSemantics(ElemTy));
> @@ -5911,9 +5911,9 @@
>      if (!Visit(E->getSubExpr()))
>        return false;
>
> -    QualType To = E->getType()->getAs<ComplexType>()->getElementType();
> +    QualType To = E->getType()->castAs<ComplexType>()->getElementType();
>      QualType From
> -      = E->getSubExpr()->getType()->getAs<ComplexType>()->getElementType();
> +      = E->getSubExpr()->getType()->castAs<ComplexType>()->getElementType();
>      Result.makeComplexFloat();
>      return HandleIntToFloatCast(Info, E, From, Result.IntReal,
>                                  To, Result.FloatReal) &&
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list