[scan-build] fix warnings emitted on Clang Sema code base

[apelete]

Summary

This partch fixes a few "Logic error" warnings of the type "Called c++
object pointer is null" reported by Clang Static Analyzer on the
following files:

• lib/Sema/SemaDeclCXX.cpp,
• lib/Sema/SemaOpenMP.cpp,
• lib/Sema/SemaOverload.cpp,
• lib/Sema/SemaInit.cpp,
• lib/Sema/SemaExprCXX.cpp,
• lib/Sema/SemaLookup.cpp,

Signed-off-by: Apelete Seketeli <[e-mail]>

[mgrang]

lib/Sema/SemaDeclCXX.cpp

438

typo: decalration ==> declaration

[apelete]

[scan-build] fix logic error warnings emitted on clang code base

Following changes were done since last revision:

• fix a typo in lib/Sema/SemaDeclCXX.cpp: decalration ==> declaration.

[mgrang]

LGTM.
Could you please check who owns the files you modified and then add a couple of reviewers. You can try "git blame <filename>" or check CODE_OWNERS.TXT file to get the owners.

[rjmccall]

lib/Sema/SemaOverload.cpp

9191

Interleaving these conditions looks strange. Please group the FromExpr-related checks next to each other.

Otherwise LGTM.

[apelete]

[scan-build] fix logic error warnings emitted on clang code base

Following changes were done since last revision:

• lib/Sema/SemaOverload.cpp: avoid interleaving (FromExpr) and (!FromExpr->isLValue()) conditions in if() statement for better readability.

[rjmccall]

LGTM, thanks.

[zaks.anna]

lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

696 ↗

(On Diff #54269)

LGTM

[apelete]

[scan-build] fix logic error warnings emitted on clang code base

Changes since last revision:

• fast forward rebase on git master branch.

[apelete]

Waiting for review, could someone please have a look at this one ?

[apelete]

[scan-build] fix warnings emitted on Clang Sema code base

Changes since last revision:

• split patch into Sema changes unit to ease review process,
• cherry-pick Sema changes from D19084,
• cherry-pick Sema changes from D19385.

lib/Sema/SemaDeclCXX.cpp

 }
 
 /// MergeCXXFunctionDecl - Merge two declarations of the same C++
 /// function, once we already know that they have the same
 /// type. Subroutine of MergeFunctionDecl. Returns true if there was an
 /// error, false otherwise.
 bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
                                 Scope *S) {
+  assert(New && "New function declaration is NULL, aborting merge.");
+  assert(Old && "Old function declaration is NULL, aborting merge.");
+
   bool Invalid = false;
 
   // The declaration context corresponding to the scope is the semantic
   // parent, unless this is a local function declaration, in which case
   // it is that surrounding function.
   DeclContext *ScopeDC = New->isLocalExternDecl()
                              ? New->getLexicalDeclContext()
                              : New->getDeclContext();

lib/Sema/SemaExprCXX.cpp

return new (Context) CXXTypeidExpr(TypeInfoType.withConst(), Operand,
                                      SourceRange(TypeidLoc, RParenLoc));
 }
 
 /// \brief Build a C++ typeid expression with an expression operand.
 ExprResult Sema::BuildCXXTypeId(QualType TypeInfoType,
                                 SourceLocation TypeidLoc,
                                 Expr *E,
                                 SourceLocation RParenLoc) {
+  assert(E && "expression operand is NULL, cannot build C++ typeid expression");
+
   bool WasEvaluated = false;
-  if (E && !E->isTypeDependent()) {
+  if (!E->isTypeDependent()) {
     if (E->getType()->isPlaceholderType()) {
       ExprResult result = CheckPlaceholderExpr(E);
       if (result.isInvalid()) return ExprError();
       E = result.get();
     }
 
     QualType T = E->getType();
     if (const RecordType *RecordT = T->getAs<RecordType>()) {

lib/Sema/SemaInit.cpp

static bool hasCompatibleArrayTypes(ASTContext &Context, const ArrayType *Dest,
   // incomplete array type and the source is a constant array type.
   return Source->isConstantArrayType() && Dest->isIncompleteArrayType();
 }
 
 static bool tryObjCWritebackConversion(Sema &S,
                                        InitializationSequence &Sequence,
                                        const InitializedEntity &Entity,
                                        Expr *Initializer) {
+  assert(Initializer && "Initializer needs to be not NULL");
+
   bool ArrayDecay = false;
   QualType ArgType = Initializer->getType();
   QualType ArgPointee;
   if (const ArrayType *ArgArrayType = S.Context.getAsArrayType(ArgType)) {
     ArrayDecay = true;
     ArgPointee = ArgArrayType->getElementType();
     ArgType = S.Context.getPointerType(ArgPointee);
   }
if (ICS.Standard.First == ICK_Array_To_Pointer ||
       AddConversionSequenceStep(LvalueICS, ICS.Standard.getToType(0));
     }
     
     AddPassByIndirectCopyRestoreStep(DestType, ShouldCopy);
   } else if (ICS.isBad()) {
     DeclAccessPair dap;
     if (isLibstdcxxPointerReturnFalseHack(S, Entity, Initializer)) {
       AddZeroInitializationStep(Entity.getType());
-    } else if (Initializer->getType() == Context.OverloadTy &&
+    } else if (Initializer && Initializer->getType() == Context.OverloadTy &&
                !S.ResolveAddressOfOverloadedFunction(Initializer, DestType,
                                                      false, dap))
       SetFailed(InitializationSequence::FK_AddressOfOverloadFailed);
-    else if (Initializer->getType()->isFunctionType() &&
+    else if (Initializer && Initializer->getType()->isFunctionType() &&
              isExprAnUnaddressableFunction(S, Initializer))
       SetFailed(InitializationSequence::FK_AddressOfUnaddressableFunction);
     else
       SetFailed(InitializationSequence::FK_ConversionFailed);
   } else {
     AddConversionSequenceStep(ICS, DestType, TopLevelOfInitList);
 
     MaybeProduceObjCObject(S, *this, Entity);

lib/Sema/SemaLookup.cpp

static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
   // Nothing to do if all decls are visible.
   if (DI == DE)
     return;
 
   llvm::SmallVector<NamedDecl*, 4> NewDecls(TC.begin(), DI);
   bool AnyVisibleDecls = !NewDecls.empty();
 
   for (/**/; DI != DE; ++DI) {
+    assert(*DI && "TypoCorrection iterator cannot be NULL");
     NamedDecl *VisibleDecl = *DI;
     if (!LookupResult::isVisible(SemaRef, *DI))
       VisibleDecl = findAcceptableDecl(SemaRef, *DI);
 
     if (VisibleDecl) {
       if (!AnyVisibleDecls) {
         // Found a visible decl, discard all hidden ones.
         AnyVisibleDecls = true;

lib/Sema/SemaOpenMP.cpp

if (DSAStack->getCurrentDirective() != OMPD_unknown &&
        DSAStack->getParentDirective() != OMPD_unknown)) {
     auto &&Info = DSAStack->isLoopControlVariable(D);
     if (Info.first ||
         (VD && VD->hasLocalStorage() &&
          isParallelOrTaskRegion(DSAStack->getCurrentDirective())) ||
         (VD && DSAStack->isForceVarCapturing()))
       return VD ? VD : Info.second;
     auto DVarPrivate = DSAStack->getTopDSA(D, DSAStack->isClauseParsingMode());
+    assert(DVarPrivate.PrivateCopy && "DSAStackTy object must be not NULL");
     if (DVarPrivate.CKind != OMPC_unknown && isOpenMPPrivate(DVarPrivate.CKind))
       return VD ? VD : cast<VarDecl>(DVarPrivate.PrivateCopy->getDecl());
     DVarPrivate = DSAStack->hasDSA(D, isOpenMPPrivate, MatchesAlways(),
                                    DSAStack->isClauseParsingMode());
     if (DVarPrivate.CKind != OMPC_unknown)
       return VD ? VD : cast<VarDecl>(DVarPrivate.PrivateCopy->getDecl());
   }
   return nullptr;
bool OpenMPIterationSpaceChecker::CheckInc(Expr *S) {
   SemaRef.Diag(S->getLocStart(), diag::err_omp_loop_not_canonical_incr)
       << S->getSourceRange() << LCDecl;
   return true;
 }
 
 static ExprResult
 tryBuildCapture(Sema &SemaRef, Expr *Capture,
                 llvm::MapVector<Expr *, DeclRefExpr *> &Captures) {
+  assert(Capture && "cannot build capture if expression is NULL");
   if (Capture->isEvaluatable(SemaRef.Context, Expr::SE_AllowSideEffects))
     return SemaRef.PerformImplicitConversion(
         Capture->IgnoreImpCasts(), Capture->getType(), Sema::AA_Converting,
         /*AllowExplicit=*/true);
   auto I = Captures.find(Capture);
   if (I != Captures.end())
     return buildCapture(SemaRef, Capture, I->second);
   DeclRefExpr *Ref = nullptr;
if (NestedLoopCount > 1) {
         SemaRef.Diag(DSA.getConstructLoc(),
                      diag::note_omp_collapse_ordered_expr)
             << 2 << CollapseLoopCountExpr->getSourceRange()
             << OrderedLoopCountExpr->getSourceRange();
       else if (CollapseLoopCountExpr)
         SemaRef.Diag(CollapseLoopCountExpr->getExprLoc(),
                      diag::note_omp_collapse_ordered_expr)
             << 0 << CollapseLoopCountExpr->getSourceRange();
-      else
+      else if (OrderedLoopCountExpr)
         SemaRef.Diag(OrderedLoopCountExpr->getExprLoc(),
                      diag::note_omp_collapse_ordered_expr)
             << 1 << OrderedLoopCountExpr->getSourceRange();
     }
     return true;
   }
   assert(For->getBody());
 

lib/Sema/SemaOverload.cpp

bool Sema::isEquivalentInternalLinkageDeclaration(const NamedDecl *A,
   }
 
   // Nothing else is sufficiently similar.
   return false;
 }
 
 void Sema::diagnoseEquivalentInternalLinkageDeclarations(
     SourceLocation Loc, const NamedDecl *D, ArrayRef<const NamedDecl *> Equiv) {
+  assert(D && "named declaration must be not NULL");
   Diag(Loc, diag::ext_equivalent_internal_linkage_decl_in_modules) << D;
 
   Module *M = getOwningModule(const_cast<NamedDecl*>(D));
   Diag(D->getLocation(), diag::note_equivalent_internal_linkage_decl)
       << !M << (M ? M->getFullModuleName() : "");
 
   for (auto *E : Equiv) {
     Module *M = getOwningModule(const_cast<NamedDecl*>(E));
if (const ObjCObjectPointerType *ToPtrTy
               FromIface->isSuperClassOf(ToIface))
             BaseToDerivedConversion = 2;
   } else if (const ReferenceType *ToRefTy = ToTy->getAs<ReferenceType>()) {
     if (ToRefTy->getPointeeType().isAtLeastAsQualifiedAs(FromTy) &&
         !FromTy->isIncompleteType() &&
         !ToRefTy->getPointeeType()->isIncompleteType() &&
         S.IsDerivedFrom(SourceLocation(), ToRefTy->getPointeeType(), FromTy)) {
       BaseToDerivedConversion = 3;
-    } else if (ToTy->isLValueReferenceType() && !FromExpr->isLValue() &&
+    } else if (FromExpr &&
+               !FromExpr->isLValue() &&
+               ToTy->isLValueReferenceType() &&
                ToTy.getNonReferenceType().getCanonicalType() ==
                FromTy.getNonReferenceType().getCanonicalType()) {
       S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_lvalue)
         << (unsigned) FnKind << FnDesc
         << (FromExpr ? FromExpr->getSourceRange() : SourceRange())
         << (unsigned) isObjectArgument << I + 1;
       MaybeEmitInheritedConstructorNote(S, Fn);
       return;