[clang][NFC] Inclusive terms: replace some uses of sanity in clang

Rewording of comments to avoid using `sanity test, sanity check`.

Reviewed By: aaron.ballman, Quuxplusone

Differential Revision: https://reviews.llvm.org/D114025

clang/include/clang/AST/Redeclarable.h

@@ -258,7 +258,8 @@ public:
 
     redecl_iterator& operator++() {
       assert(Current && "Advancing while iterator has reached end");
-      // Sanity check to avoid infinite loop on invalid redecl chain.
+      // Make sure we don't infinitely loop on an invalid redecl chain. This
+      // should never happen.
       if (Current->isFirstDecl()) {
         if (PassedFirst) {
           assert(0 && "Passed first decl twice, invalid redecl chain!");

clang/include/clang/Analysis/CFG.h

@@ -515,7 +515,7 @@ public:
     /// of the most derived class while we're in the base class.
     VirtualBaseBranch,
 
-    /// Number of different kinds, for sanity checks. We subtract 1 so that
+    /// Number of different kinds, for validity checks. We subtract 1 so that
     /// to keep receiving compiler warnings when we don't cover all enum values
     /// in a switch.
     NumKindsMinusOne = VirtualBaseBranch

clang/include/clang/CodeGen/CGFunctionInfo.h

@@ -250,7 +250,7 @@ public:
   static ABIArgInfo getCoerceAndExpand(llvm::StructType *coerceToType,
                                        llvm::Type *unpaddedCoerceToType) {
 #ifndef NDEBUG
-    // Sanity checks on unpaddedCoerceToType.
+    // Check that unpaddedCoerceToType has roughly the right shape.
 
     // Assert that we only have a struct type if there are multiple elements.
     auto unpaddedStruct = dyn_cast<llvm::StructType>(unpaddedCoerceToType);

clang/include/clang/Sema/Lookup.h

@@ -319,7 +319,7 @@ public:
   }
 
   LookupResultKind getResultKind() const {
-    assert(sanity());
+    assert(checkDebugAssumptions());
     return ResultKind;
   }
 
@@ -706,10 +706,9 @@ private:
   void addDeclsFromBasePaths(const CXXBasePaths &P);
   void configure();
 
-  // Sanity checks.
+  bool checkDebugAssumptions() const;
-  bool sanity() const;
 
-  bool sanityCheckUnresolved() const {
+  bool checkUnresolved() const {
     for (iterator I = begin(), E = end(); I != E; ++I)
       if (isa<UnresolvedUsingValueDecl>((*I)->getUnderlyingDecl()))
         return true;

clang/lib/Analysis/BodyFarm.cpp

@@ -790,9 +790,8 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
     }
   }
 
-  // Sanity check that the property is the same type as the ivar, or a
+  // We expect that the property is the same type as the ivar, or a reference to
-  // reference to it, and that it is either an object pointer or trivially
+  // it, and that it is either an object pointer or trivially copyable.
-  // copyable.
   if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
                                   Prop->getType().getNonReferenceType()))
     return nullptr;

clang/lib/Analysis/RetainSummaryManager.cpp

@@ -791,7 +791,7 @@ RetainSummaryManager::getUnarySummary(const FunctionType* FT,
   // Unary functions have no arg effects by definition.
   ArgEffects ScratchArgs(AF.getEmptyMap());
 
-  // Sanity check that this is *really* a unary function.  This can
+  // Verify that this is *really* a unary function.  This can
   // happen if people do weird things.
   const FunctionProtoType* FTP = dyn_cast<FunctionProtoType>(FT);
   if (!FTP || FTP->getNumParams() != 1)

clang/lib/Basic/DiagnosticIDs.cpp

@@ -692,7 +692,7 @@ void DiagnosticIDs::getAllDiagnostics(diag::Flavor Flavor,
 StringRef DiagnosticIDs::getNearestOption(diag::Flavor Flavor,
                                           StringRef Group) {
   StringRef Best;
-  unsigned BestDistance = Group.size() + 1; // Sanity threshold.
+  unsigned BestDistance = Group.size() + 1; // Maximum threshold.
   for (const WarningOption &O : OptionTable) {
     // Don't suggest ignored warning flags.
     if (!O.Members && !O.SubGroups)

clang/lib/Basic/SourceManager.cpp

@@ -59,12 +59,10 @@ unsigned ContentCache::getSizeBytesMapped() const {
 /// Returns the kind of memory used to back the memory buffer for
 /// this content cache.  This is used for performance analysis.
 llvm::MemoryBuffer::BufferKind ContentCache::getMemoryBufferKind() const {
-  assert(Buffer);
+  if (Buffer == nullptr) {
-
+    assert(0 && "Buffer should never be null");
-  // Should be unreachable, but keep for sanity.
-  if (!Buffer)
     return llvm::MemoryBuffer::MemoryBuffer_Malloc;
-
+  }
   return Buffer->getBufferKind();
 }
 
@@ -864,7 +862,6 @@ FileID SourceManager::getFileIDLocal(SourceLocation::UIntTy SLocOffset) const {
 /// This function knows that the SourceLocation is in a loaded buffer, not a
 /// local one.
 FileID SourceManager::getFileIDLoaded(SourceLocation::UIntTy SLocOffset) const {
-  // Sanity checking, otherwise a bug may lead to hanging in release build.
   if (SLocOffset < CurrentLoadedOffset) {
     assert(0 && "Invalid SLocOffset or bad function choice");
     return FileID();
@@ -909,7 +906,6 @@ FileID SourceManager::getFileIDLoaded(SourceLocation::UIntTy SLocOffset) const {
     ++NumProbes;
 
     if (E.getOffset() > SLocOffset) {
-      // Sanity checking, otherwise a bug may lead to hanging in release build.
       if (GreaterIndex == MiddleIndex) {
         assert(0 && "binary search missed the entry");
         return FileID();
@@ -925,7 +921,6 @@ FileID SourceManager::getFileIDLoaded(SourceLocation::UIntTy SLocOffset) const {
       return Res;
     }
 
-    // Sanity checking, otherwise a bug may lead to hanging in release build.
     if (LessIndex == MiddleIndex) {
       assert(0 && "binary search missed the entry");
       return FileID();

clang/lib/Driver/ToolChains/Clang.cpp

@@ -4303,7 +4303,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   const Driver &D = TC.getDriver();
   ArgStringList CmdArgs;
 
-  // Check number of inputs for sanity. We need at least one input.
   assert(Inputs.size() >= 1 && "Must have at least one input.");
   // CUDA/HIP compilation may have multiple inputs (source file + results of
   // device-side compilations). OpenMP device jobs also take the host IR as a

clang/lib/Format/Format.cpp

@@ -2579,7 +2579,7 @@ tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
   // doesn't have hidden dependencies
   // (http://llvm.org/docs/CodingStandards.html#include-style).
   //
-  // FIXME: Do some sanity checking, e.g. edit distance of the base name, to fix
+  // FIXME: Do some validation, e.g. edit distance of the base name, to fix
   // cases where the first #include is unlikely to be the main header.
   tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
   bool FirstIncludeBlock = true;

clang/lib/Frontend/FrontendActions.cpp

@@ -842,7 +842,7 @@ void PrintPreprocessedAction::ExecuteAction() {
       const char *next = (cur != end) ? cur + 1 : end;
 
       // Limit ourselves to only scanning 256 characters into the source
-      // file.  This is mostly a sanity check in case the file has no
+      // file.  This is mostly a check in case the file has no
       // newlines whatsoever.
       if (end - cur > 256)
         end = cur + 256;

clang/lib/Sema/SemaChecking.cpp

@@ -5532,8 +5532,8 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
 
   // For an arithmetic operation, the implied arithmetic must be well-formed.
   if (Form == Arithmetic) {
-    // gcc does not enforce these rules for GNU atomics, but we do so for
+    // GCC does not enforce these rules for GNU atomics, but we do, because if
-    // sanity.
+    // we didn't it would be very confusing. FIXME:  For whom? How so?
     auto IsAllowedValueType = [&](QualType ValType) {
       if (ValType->isIntegerType())
         return true;
@@ -5574,7 +5574,8 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
   if (!IsC11 && !AtomTy.isTriviallyCopyableType(Context) &&
       !AtomTy->isScalarType()) {
     // For GNU atomics, require a trivially-copyable type. This is not part of
-    // the GNU atomics specification, but we enforce it for sanity.
+    // the GNU atomics specification, but we enforce it, because if we didn't it
+    // would be very confusing. FIXME:  For whom? How so?
     Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_trivial_copy)
         << Ptr->getType() << Ptr->getSourceRange();
     return ExprError();

clang/lib/Sema/SemaDecl.cpp

@@ -12621,8 +12621,9 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
 }
 
 /// ActOnInitializerError - Given that there was an error parsing an
-/// initializer for the given declaration, try to return to some form
+/// initializer for the given declaration, try to at least re-establish
-/// of sanity.
+/// invariants such as whether a variable's type is either dependent or
+/// complete.
 void Sema::ActOnInitializerError(Decl *D) {
   // Our main concern here is re-establishing invariants like "a
   // variable's type is either dependent or complete".
@@ -15997,8 +15998,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
 
     // It's okay to have a tag decl in the same scope as a typedef
     // which hides a tag decl in the same scope.  Finding this
-    // insanity with a redeclaration lookup can only actually happen
+    // with a redeclaration lookup can only actually happen in C++.
-    // in C++.
     //
     // This is also okay for elaborated-type-specifiers, which is
     // technically forbidden by the current standard but which is

clang/lib/Sema/SemaDeclCXX.cpp

@@ -9172,7 +9172,8 @@ bool SpecialMemberDeletionInfo::shouldDeleteForField(FieldDecl *FD) {
       }
 
       // Don't check the implicit member of the anonymous union type.
-      // This is technically non-conformant, but sanity demands it.
+      // This is technically non-conformant but supported, and we have a
+      // diagnostic for this elsewhere.
       return false;
     }
 
@@ -12256,7 +12257,7 @@ NamedDecl *Sema::BuildUsingDeclaration(
   // Unlike most lookups, we don't always want to hide tag
   // declarations: tag names are visible through the using declaration
   // even if hidden by ordinary names, *except* in a dependent context
-  // where it's important for the sanity of two-phase lookup.
+  // where they may be used by two-phase lookup.
   if (!IsInstantiation)
     R.setHideTags(false);
 

clang/lib/Sema/SemaExpr.cpp

@@ -11185,7 +11185,6 @@ QualType Sema::CheckShiftOperands(ExprResult &LHS, ExprResult &RHS,
       isScopedEnumerationType(RHSType)) {
     return InvalidOperands(Loc, LHS, RHS);
   }
-  // Sanity-check shift operands
   DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
 
   // "The type of the result is that of the promoted left operand."

clang/lib/Sema/SemaExprCXX.cpp

@@ -1507,8 +1507,9 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
     ElemTy = Context.getBaseElementType(Ty);
   }
 
-  // There doesn't seem to be an explicit rule against this but sanity demands
+  // Only construct objects with object types.
-  // we only construct objects with object types.
+  // There doesn't seem to be an explicit rule for this but functions are
+  // not objects, so they cannot take initializers.
   if (Ty->isFunctionType())
     return ExprError(Diag(TyBeginLoc, diag::err_init_for_function_type)
                        << Ty << FullRange);

clang/lib/Sema/SemaLookup.cpp

@@ -324,14 +324,14 @@ void LookupResult::configure() {
   }
 }
 
-bool LookupResult::sanity() const {
+bool LookupResult::checkDebugAssumptions() const {
   // This function is never called by NDEBUG builds.
   assert(ResultKind != NotFound || Decls.size() == 0);
   assert(ResultKind != Found || Decls.size() == 1);
   assert(ResultKind != FoundOverloaded || Decls.size() > 1 ||
          (Decls.size() == 1 &&
           isa<FunctionTemplateDecl>((*begin())->getUnderlyingDecl())));
-  assert(ResultKind != FoundUnresolvedValue || sanityCheckUnresolved());
+  assert(ResultKind != FoundUnresolvedValue || checkUnresolved());
   assert(ResultKind != Ambiguous || Decls.size() > 1 ||
          (Decls.size() == 1 && (Ambiguity == AmbiguousBaseSubobjects ||
                                 Ambiguity == AmbiguousBaseSubobjectTypes)));

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

@@ -1517,7 +1517,7 @@ void CStringChecker::evalStrcat(CheckerContext &C, const CallExpr *CE) const {
 }
 
 void CStringChecker::evalStrncat(CheckerContext &C, const CallExpr *CE) const {
-  //char *strncat(char *restrict s1, const char *restrict s2, size_t n);
+  // char *strncat(char *restrict s1, const char *restrict s2, size_t n);
   evalStrcpyCommon(C, CE,
                    /* ReturnEnd = */ false,
                    /* IsBounded = */ true,
@@ -2069,8 +2069,8 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE,
 }
 
 void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const {
-  //char *strsep(char **stringp, const char *delim);
+  // char *strsep(char **stringp, const char *delim);
-  // Sanity: does the search string parameter match the return type?
+  // Verify whether the search string parameter matches the return type.
   SourceArgExpr SearchStrPtr = {CE->getArg(0), 0};
 
   QualType CharPtrTy = SearchStrPtr.Expression->getType()->getPointeeType();

clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp

@@ -160,7 +160,7 @@ static bool isInMIGCall(CheckerContext &C) {
   if (Optional<AnyCall> AC = AnyCall::forDecl(D)) {
     // Even though there's a Sema warning when the return type of an annotated
     // function is not a kern_return_t, this warning isn't an error, so we need
-    // an extra sanity check here.
+    // an extra check here.
     // FIXME: AnyCall doesn't support blocks yet, so they remain unchecked
     // for now.
     if (!AC->getReturnType(C.getASTContext())

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

@@ -155,7 +155,7 @@ class StdLibraryFunctionsChecker
   protected:
     ArgNo ArgN; // Argument to which we apply the constraint.
 
-    /// Do polymorphic sanity check on the constraint.
+    /// Do polymorphic validation check on the constraint.
     virtual bool checkSpecificValidity(const FunctionDecl *FD) const {
       return true;
     }
@@ -527,8 +527,8 @@ class StdLibraryFunctionsChecker
     }
 
   private:
-    // Once we know the exact type of the function then do sanity check on all
+    // Once we know the exact type of the function then do validation check on
-    // the given constraints.
+    // all the given constraints.
     bool validateByConstraints(const FunctionDecl *FD) const {
       for (const ConstraintSet &Case : CaseConstraints)
         for (const ValueConstraintPtr &Constraint : Case)

clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

@@ -182,8 +182,7 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
   ProgramStateRef state = C.getState();
 
   if (CE->getNumArgs() < MinArgCount) {
-    // The frontend should issue a warning for this case, so this is a sanity
+    // The frontend should issue a warning for this case. Just return.
-    // check.
     return;
   } else if (CE->getNumArgs() == MaxArgCount) {
     const Expr *Arg = CE->getArg(CreateModeArgIndex);
@@ -366,7 +365,7 @@ void UnixAPIPortabilityChecker::BasicAllocationCheck(CheckerContext &C,
                                                      const unsigned numArgs,
                                                      const unsigned sizeArg,
                                                      const char *fn) const {
-  // Sanity check for the correct number of arguments
+  // Check for the correct number of arguments.
   if (CE->getNumArgs() != numArgs)
     return;
 

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

@@ -1670,9 +1670,10 @@ PathDiagnosticPieceRef TrackConstraintBRVisitor::VisitNode(
   if (isUnderconstrained(PrevN)) {
     IsSatisfied = true;
 
-    // As a sanity check, make sure that the negation of the constraint
+    // At this point, the negation of the constraint should be infeasible. If it
-    // was infeasible in the current state.  If it is feasible, we somehow
+    // is feasible, make sure that the negation of the constrainti was
-    // missed the transition point.
+    // infeasible in the current state.  If it is feasible, we somehow missed
+    // the transition point.
     assert(!isUnderconstrained(N));
 
     // We found the transition point for the constraint.  We now need to

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -326,8 +326,8 @@ ProgramStateRef ExprEngine::createTemporaryRegionIfNeeded(
     }
     Result = InitWithAdjustments;
   } else {
-    // We need to create a region no matter what. For sanity, make sure we don't
+    // We need to create a region no matter what. Make sure we don't try to
-    // try to stuff a Loc into a non-pointer temporary region.
+    // stuff a Loc into a non-pointer temporary region.
     assert(!InitValWithAdjustments.getAs<Loc>() ||
            Loc::isLocType(Result->getType()) ||
            Result->getType()->isMemberPointerType());

clang/lib/StaticAnalyzer/Core/Store.cpp

@@ -249,7 +249,7 @@ static bool regionMatchesCXXRecordType(SVal V, QualType Ty) {
 }
 
 SVal StoreManager::evalDerivedToBase(SVal Derived, const CastExpr *Cast) {
-  // Sanity check to avoid doing the wrong thing in the face of
+  // Early return to avoid doing the wrong thing in the face of
   // reinterpret_cast.
   if (!regionMatchesCXXRecordType(Derived, Cast->getSubExpr()->getType()))
     return UnknownVal();

clang/lib/Tooling/Syntax/Tree.cpp

@@ -126,7 +126,7 @@ void syntax::Tree::replaceChildRangeLowLevel(Node *Begin, Node *End,
   for (auto *N = New; N; N = N->NextSibling) {
     assert(N->Parent == nullptr);
     assert(N->getRole() != NodeRole::Detached && "Roles must be set");
-    // FIXME: sanity-check the role.
+    // FIXME: validate the role.
   }
 
   auto Reachable = [](Node *From, Node *N) {