r175594 - Replace SVal llvm::cast support to be well-defined.

[Jordan Rose]

Thanks, David!

Is it all right if I add Optional to clang/Basic/LLVM.h, so that it doesn't have to appear everywhere?

A few comments inline.


On Feb 19, 2013, at 21:52 , David Blaikie <dblaikie at gmail.com> wrote:

> Author: dblaikie
> Date: Tue Feb 19 23:52:05 2013
> New Revision: 175594
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=175594&view=rev
> Log:
> Replace SVal llvm::cast support to be well-defined.
> 
> See r175462 for another example/more details.
> 
> Modified:
>    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
>    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
>    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
>    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
>    cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
> 


> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h?rev=175594&r1=175593&r2=175594&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h Tue Feb 19 23:52:05 2013
> @@ -69,6 +69,25 @@ protected:
> public:
>   explicit SVal() : Data(0), Kind(0) {}
> 
> +  template<typename T>
> +  T castAs() const {
> +    assert(T::isType(*this));
> +    T t;
> +    SVal& sv = t;
> +    sv = *this;
> +    return t;
> +  }
> +
> +  template<typename T>
> +  llvm::Optional<T> getAs() const {
> +    if (!T::isType(*this))
> +      return llvm::Optional<T>();
> +    T t;
> +    SVal& sv = t;
> +    sv = *this;
> +    return t;
> +  }

Bikeshedding to isKind, instead of isType, since "type" is such an overloaded word in Clang already.

Also, capital variable names, and ampersand on the variable name rather than the type.

I guess Val->SVal::operator=(*this) isn't really cleaner than the reference trick, huh. A bunch of these types didn't have default constructors, though, so I'm wondering if adding T(const SVal &) constructors makes more sense. Eh.


>  
> -  bool isExpression() {
> +  bool isExpression() const {
>     return !isa<SymbolData>(getSymbol());
>   }

Heh, thanks.

>   if (originalRegion != R) {
>     if (Optional<SVal> OV = B.getDefaultBinding(R)) {
> -      if (const nonloc::LazyCompoundVal *V =
> -            dyn_cast<nonloc::LazyCompoundVal>(OV.getPointer()))
> +      if (llvm::Optional<nonloc::LazyCompoundVal> V =
> +              OV.getPointer()->getAs<nonloc::LazyCompoundVal>())

This is going through another Optional, so you should be able to just use OV->getAs now.


> @@ -1669,9 +1670,8 @@ NonLoc RegionStoreManager::createLazyBin
>   // If we already have a lazy binding, and it's for the whole structure,
>   // don't create a new lazy binding.
>   if (Optional<SVal> V = B.getDefaultBinding(R)) {
> -    const nonloc::LazyCompoundVal *LCV =
> -      dyn_cast<nonloc::LazyCompoundVal>(V.getPointer());
> -    if (LCV) {
> +    if (llvm::Optional<nonloc::LazyCompoundVal> LCV =
> +            V.getPointer()->getAs<nonloc::LazyCompoundVal>()) {

Ditto.