ZarkoCA updated this revision to Diff 388494.
ZarkoCA added a comment.
- Add FIXME to comments and fix grammar on one comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114025/new/
https://reviews.llvm.org/D114025
Files:
clang/include/clang/AST/Redeclarable.h
clang/include/clang/Analysis/CFG.h
clang/include/clang/CodeGen/CGFunctionInfo.h
clang/include/clang/Sema/Lookup.h
clang/lib/Analysis/BodyFarm.cpp
clang/lib/Analysis/RetainSummaryManager.cpp
clang/lib/Basic/DiagnosticIDs.cpp
clang/lib/Basic/SourceManager.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Format/Format.cpp
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/Store.cpp
clang/lib/Tooling/Syntax/Tree.cpp
Index: clang/lib/Tooling/Syntax/Tree.cpp
===================================================================
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -126,7 +126,7 @@
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) {
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -249,7 +249,7 @@
}
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();
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -326,8 +326,8 @@
}
Result = InitWithAdjustments;
} else {
- // We need to create a region no matter what. For sanity, make sure we don't
- // try to stuff a Loc into a non-pointer temporary region.
+ // We need to create a region no matter what. Make sure we don't try to
+ // stuff a Loc into a non-pointer temporary region.
assert(!InitValWithAdjustments.getAs<Loc>() ||
Loc::isLocType(Result->getType()) ||
Result->getType()->isMemberPointerType());
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1670,9 +1670,10 @@
if (isUnderconstrained(PrevN)) {
IsSatisfied = true;
- // As a sanity check, make sure that the negation of the constraint
- // was infeasible in the current state. If it is feasible, we somehow
- // missed the transition point.
+ // At this point, the negation of the constraint should be infeasible. If it
+ // is feasible, make sure that the negation of the constrainti was
+ // 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
Index: clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -182,8 +182,7 @@
ProgramStateRef state = C.getState();
if (CE->getNumArgs() < MinArgCount) {
- // The frontend should issue a warning for this case, so this is a sanity
- // check.
+ // The frontend should issue a warning for this case. Just return.
return;
} else if (CE->getNumArgs() == MaxArgCount) {
const Expr *Arg = CE->getArg(CreateModeArgIndex);
@@ -366,7 +365,7 @@
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;
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -155,7 +155,7 @@
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 @@
}
private:
- // Once we know the exact type of the function then do sanity check on all
- // the given constraints.
+ // Once we know the exact type of the function then do validation check on
+ // all the given constraints.
bool validateByConstraints(const FunctionDecl *FD) const {
for (const ConstraintSet &Case : CaseConstraints)
for (const ValueConstraintPtr &Constraint : Case)
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -160,7 +160,7 @@
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())
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2070,7 +2070,7 @@
void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const {
//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();
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -324,14 +324,14 @@
}
}
-bool LookupResult::sanity() const {
+bool LookupResult::check() 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)));
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1507,8 +1507,9 @@
ElemTy = Context.getBaseElementType(Ty);
}
- // There doesn't seem to be an explicit rule against this but sanity demands
- // we only construct objects with object types.
+ // 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);
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11185,7 +11185,6 @@
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."
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -9172,7 +9172,7 @@
}
// 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 check it anyway.
return false;
}
@@ -12256,7 +12256,7 @@
// 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 it's important for performing a valid two-phase lookup.
if (!IsInstantiation)
R.setHideTags(false);
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12621,8 +12621,9 @@
}
/// ActOnInitializerError - Given that there was an error parsing an
-/// initializer for the given declaration, try to return to some form
-/// of sanity.
+/// initializer for the given declaration, try to at least re-establish
+/// 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 @@
// 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
- // in C++.
+ // with a redeclaration lookup can only actually happen in C++.
//
// This is also okay for elaborated-type-specifiers, which is
// technically forbidden by the current standard but which is
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -5532,8 +5532,8 @@
// 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
- // sanity.
+ // GCC does not enforce these rules for GNU atomics, but we do, because if
+ // 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 @@
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();
Index: clang/lib/Frontend/FrontendActions.cpp
===================================================================
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -842,7 +842,7 @@
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;
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2579,7 +2579,7 @@
// 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;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4303,7 +4303,6 @@
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
Index: clang/lib/Basic/SourceManager.cpp
===================================================================
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -59,12 +59,10 @@
/// 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);
-
- // Should be unreachable, but keep for sanity.
- if (!Buffer)
+ if (Buffer == nullptr) {
+ assert(0 && "Buffer should never be null");
return llvm::MemoryBuffer::MemoryBuffer_Malloc;
-
+ }
return Buffer->getBufferKind();
}
@@ -864,7 +862,6 @@
/// 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 @@
++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 @@
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();
Index: clang/lib/Basic/DiagnosticIDs.cpp
===================================================================
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -692,7 +692,7 @@
StringRef DiagnosticIDs::getNearestOption(diag::Flavor Flavor,
StringRef Group) {
StringRef Best;
- unsigned BestDistance = Group.size() + 1; // Sanity threshold.
+ unsigned BestDistance = Group.size() + 1; // Minumum threshold.
for (const WarningOption &O : OptionTable) {
// Don't suggest ignored warning flags.
if (!O.Members && !O.SubGroups)
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===================================================================
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -791,7 +791,7 @@
// 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)
Index: clang/lib/Analysis/BodyFarm.cpp
===================================================================
--- clang/lib/Analysis/BodyFarm.cpp
+++ clang/lib/Analysis/BodyFarm.cpp
@@ -790,9 +790,8 @@
}
}
- // Sanity check that the property is the same type as the ivar, or a
- // reference to it, and that it is either an object pointer or trivially
- // copyable.
+ // We expect that the property is the same type as the ivar, or a reference to
+ // it, and that it is either an object pointer or trivially copyable.
if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
Prop->getType().getNonReferenceType()))
return nullptr;
Index: clang/include/clang/Sema/Lookup.h
===================================================================
--- clang/include/clang/Sema/Lookup.h
+++ clang/include/clang/Sema/Lookup.h
@@ -319,7 +319,7 @@
}
LookupResultKind getResultKind() const {
- assert(sanity());
+ assert(check());
return ResultKind;
}
@@ -706,10 +706,9 @@
void addDeclsFromBasePaths(const CXXBasePaths &P);
void configure();
- // Sanity checks.
- bool sanity() const;
+ bool check() const;
- bool sanityCheckUnresolved() const {
+ bool checkUnresolved() const {
for (iterator I = begin(), E = end(); I != E; ++I)
if (isa<UnresolvedUsingValueDecl>((*I)->getUnderlyingDecl()))
return true;
Index: clang/include/clang/CodeGen/CGFunctionInfo.h
===================================================================
--- clang/include/clang/CodeGen/CGFunctionInfo.h
+++ clang/include/clang/CodeGen/CGFunctionInfo.h
@@ -250,7 +250,7 @@
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);
Index: clang/include/clang/Analysis/CFG.h
===================================================================
--- clang/include/clang/Analysis/CFG.h
+++ clang/include/clang/Analysis/CFG.h
@@ -515,7 +515,7 @@
/// 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
Index: clang/include/clang/AST/Redeclarable.h
===================================================================
--- clang/include/clang/AST/Redeclarable.h
+++ clang/include/clang/AST/Redeclarable.h
@@ -258,7 +258,8 @@
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 on invalid redecl chain. This
+ // should never happen
if (Current->isFirstDecl()) {
if (PassedFirst) {
assert(0 && "Passed first decl twice, invalid redecl chain!");
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
