[NFC][CLANG] Fix static analyzer tool remarks about unchecked return values

[Manna]

Summary

Reported by Coverity:

Unchecked return value
If the function returns an error value, the error value may be mistaken for a normal value.

1. Inside "Parser.cpp" file, in clang::Parser::ParseKNRParamDeclarations(clang::Declarator &): Value returned from a function is not checked for errors before being used.

check_return: Calling TryConsumeToken without checking return value (as is done elsewhere 75 out of 86 times).

2. Inside "CallGraph.h" file, in clang::CallGraph::addToCallGraph(clang::Decl *): Value returned from a function is not checked for errors before being used.

check_return: Calling TraverseDecl without checking return value (as is done elsewhere 22 out of 23 times).

3. Inside "TokenAnnotator.cpp" file ,in clang::​format::​<unnamed>::​AnnotatingParser::​consumeToken(): Value returned from a function is not checked for errors before being used.
   
   check_return: Calling consumeToken without checking return value (as is done elsewhere 6 out of 7 times).

4. Inside "UnsafeBufferUsage.cpp" file, In clang::​ast_matchers::​MatchDescendantVisitor::​findMatch(clang::​DynTypedNode const &): Value returned from a function is not checked for errors before being used.

check_return: Calling TraverseStmt without checking return value (as is done elsewhere 485 out of 489 times)

5. Inside "ItaniumDemangle.h" file, in llvm::​itanium_demangle::​AbstractManglingParser<llvm::​itanium_demangle::​ManglingParser<<unnamed>::​CanonicalizerAllocator>, <unnamed>::​CanonicalizerAllocator>::​parseFunctionType(): Value returned from a function is not checked for errors before being used.
   
   check_return: Calling consumeIf without checking return value (as is done elsewhere 47 out of 50 times).

6. Inside "ItaniumDemangle.h" file, in llvm::​itanium_demangle::​AbstractManglingParser<llvm::​itanium_demangle::​ManglingParser<<unnamed>::​CanonicalizerAllocator>, <unnamed>::​CanonicalizerAllocator>::​parseBaseUnresolvedName(): Value returned from a function is not checked for errors before being used.

check_return: Calling consumeIf without checking return value (as is done elsewhere 47 out of 50 times).

7. Inside "ItaniumDemangle.h" file, in llvm::​itanium_demangle::​AbstractManglingParser<llvm::​itanium_demangle::​ManglingParser<llvm::​esimd::​SimpleAllocator>, llvm::​esimd::​SimpleAllocator>::​parseNumber(bool): Value returned from a function is not checked for errors before being used.

check_return: Calling consumeIf without checking return value (as is done elsewhere 95 out of 102 times).

8. Inside "ExprConstant.cpp" file, in clang::​Expr::​isPotentialConstantExprUnevaluated(clang::​Expr *, clang::​FunctionDecl const *, llvm::​SmallVectorImpl<std::​pair<clang::​SourceLocation, clang::​PartialDiagnostic>> &): Value returned from a function is not checked for errors before being used.

check_return: Calling Evaluate without checking return value (as is done elsewhere 23 out of 27 times).

9. Inside "ASTMatchFinder.cpp" file, in clang::​ast_matchers::​internal::​<unnamed>::​MatchASTVisitor::​dataTraverseNode(clang::​Stmt *, llvm::​SmallVectorImpl<llvm::​PointerIntPair<clang::​Stmt *, 1u, bool, llvm::​PointerLikeTypeTraits<clang::​Stmt *>, llvm::​PointerIntPairInfo<clang::​Stmt *, 1u, llvm::​PointerLikeTypeTraits<clang::​Stmt *>>>> *): Value returned from a function is not checked for errors before being used.

check_return: Calling TraverseDecl without checking return value (as is done elsewhere 29 out of 33 times).

10. Inside "ParseObjc.cpp" file, in clang::​Parser::​isStartOfObjCClassMessageMissingOpenBracket(): Value returned from a function is not checked for errors before being used.

check_return: Calling TryAnnotateTypeOrScopeToken without checking return value (as is done elsewhere 19 out of 21 times).

[erichkeane]

I don't see much value in these changes. In the cases I know anything about, the change in state (or out params) is the error checking that we need. The casts add nothing but noise, so I don't think these are good changes.

[Manna]

In D147791#4251497, @erichkeane wrote:

I don't see much value in these changes. In the cases I know anything about, the change in state (or out params) is the error checking that we need. The casts add nothing but noise, so I don't think these are good changes.

Thanks @erichkeane for reviews and feedback. I agree with you that all changes will silence the static analyzer tool but will not make any big impact. I will close them as a False positive.

[erichkeane]

That sounds appropriate to me.

clang/include/clang/Analysis/CallGraph.h

public:
   CallGraph();
   ~CallGraph();
 
   /// Populate the call graph with the functions in the given
   /// declaration.
   ///
   /// Recursively walks the declaration to find all the dependent Decls as well.
   void addToCallGraph(Decl *D) {
-    TraverseDecl(D);
+    (void)TraverseDecl(D);
   }
 
   /// Determine if a declaration should be included in the graph.
   static bool includeInGraph(const Decl *D);
 
   /// Determine if a declaration should be included in the graph for the 
   /// purposes of being a callee. This is similar to includeInGraph except
   /// it permits declarations, not just definitions.

clang/lib/AST/ExprConstant.cpp

EvalInfo Info(FD->getASTContext(), Status,
                 EvalInfo::EM_ConstantExpressionUnevaluated);
   Info.InConstantContext = true;
   Info.CheckingPotentialConstantExpression = true;
 
   // Fabricate a call stack frame to give the arguments a plausible cover story.
   CallStackFrame Frame(Info, SourceLocation(), FD, /*This*/ nullptr, CallRef());
 
   APValue ResultScratch;
-  Evaluate(ResultScratch, Info, E);
+  (void)Evaluate(ResultScratch, Info, E);
   return Diags.empty();
 }
 
 bool Expr::tryEvaluateObjectSize(uint64_t &Result, ASTContext &Ctx,
                                  unsigned Type) const {
   if (!getType()->isPointerType())
     return false;
 

clang/lib/ASTMatchers/ASTMatchFinder.cpp

if (auto *RF = dyn_cast<CXXForRangeStmt>(S)) {
           if (Expr *RequiresClause = TPL->getRequiresClause()) {
             TraverseStmt(RequiresClause);
           }
         }
 
         if (LE->hasExplicitParameters()) {
           // Visit parameters.
           for (ParmVarDecl *Param : Proto.getParams())
-            TraverseDecl(Param);
+            (void)TraverseDecl(Param);
         }
 
         const auto *T = Proto.getTypePtr();
         for (const auto &E : T->exceptions())
           TraverseType(E);
 
         if (Expr *NE = T->getNoexceptExpr())
           TraverseStmt(NE, Queue);

clang/lib/Analysis/UnsafeBufferUsage.cpp

MatchDescendantVisitor(const internal::DynTypedMatcher *Matcher,
       : Matcher(Matcher), Finder(Finder), Builder(Builder), Bind(Bind),
         Matches(false) {}
 
   // Returns true if a match is found in a subtree of `DynNode`, which belongs
   // to the same callable of `DynNode`.
   bool findMatch(const DynTypedNode &DynNode) {
     Matches = false;
     if (const Stmt *StmtNode = DynNode.get<Stmt>()) {
-      TraverseStmt(const_cast<Stmt *>(StmtNode));
+      (void)TraverseStmt(const_cast<Stmt *>(StmtNode));
       *Builder = ResultBindings;
       return Matches;
     }
     return false;
   }
 
   // The following are overriding methods from the base visitor class.
   // They are public only to allow CRTP to work. They are *not *part

clang/lib/Format/TokenAnnotator.cpp

case tok::kw_operator:
         if (CurrentToken->isOneOf(tok::star, tok::amp))
           CurrentToken->setType(TT_PointerOrReference);
         auto Next = CurrentToken->getNextNonComment();
         if (!Next)
           break;
         if (Next->is(tok::less))
           next();
         else
-          consumeToken();
+          (void)consumeToken();
         if (!CurrentToken)
           break;
         auto Previous = CurrentToken->getPreviousNonComment();
         assert(Previous);
         if (CurrentToken->is(tok::comma) && Previous->isNot(tok::kw_operator))
           break;
         if (Previous->isOneOf(TT_BinaryOperator, TT_UnaryOperator, tok::comma,
                               tok::star, tok::arrow, tok::amp, tok::ampamp) ||

clang/lib/Parse/ParseObjc.cpp

bool Parser::isStartOfObjCClassMessageMissingOpenBracket() {
   else
     return false;
 
   // FIXME: Should not be querying properties of types from the parser.
   if (Type.isUsable() && Type.get().get()->isObjCObjectOrInterfaceType()) {
     const Token &AfterNext = GetLookAheadToken(2);
     if (AfterNext.isOneOf(tok::colon, tok::r_square)) {
       if (Tok.is(tok::identifier))
-        TryAnnotateTypeOrScopeToken();
+        (void)TryAnnotateTypeOrScopeToken();
 
       return Tok.is(tok::annot_typename);
     }
   }
 
   return false;
 }
 

clang/lib/Parse/Parser.cpp

while (isDeclarationSpecifier(ImplicitTypenameContext::No)) {
 
     // Consume ';' and continue parsing.
     if (!ExpectAndConsumeSemi(diag::err_expected_semi_declaration))
       continue;
 
     // Otherwise recover by skipping to next semi or mandatory function body.
     if (SkipUntil(tok::l_brace, StopAtSemi | StopBeforeMatch))
       break;
-    TryConsumeToken(tok::semi);
+    (void)TryConsumeToken(tok::semi);
   }
 
   // The actions module must verify that all arguments were declared.
   Actions.ActOnFinishKNRParamDeclarations(getCurScope(), D, Tok.getLocation());
 }
 
 
 /// ParseAsmStringLiteral - This is just a normal string-literal, but is not

llvm/include/llvm/Demangle/ItaniumDemangle.h

 template <typename Derived, typename Alloc>
 Node *AbstractManglingParser<Derived, Alloc>::parseBaseUnresolvedName() {
   if (std::isdigit(look()))
     return getDerived().parseSimpleId();
 
   if (consumeIf("dn"))
     return getDerived().parseDestructorName();
 
-  consumeIf("on");
+  (void)consumeIf("on");
 
   Node *Oper = getDerived().parseOperatorName(/*NameState=*/nullptr);
   if (Oper == nullptr)
     return nullptr;
   if (look() == 'I') {
     Node *TA = getDerived().parseTemplateArgs();
     if (TA == nullptr)
       return nullptr;

 }
 
 // <number> ::= [n] <non-negative decimal integer>
 template <typename Alloc, typename Derived>
 StringView
 AbstractManglingParser<Alloc, Derived>::parseNumber(bool AllowNegative) {
   const char *Tmp = First;
   if (AllowNegative)
-    consumeIf('n');
+    (void)consumeIf('n');
   if (numLeft() == 0 || !std::isdigit(*First))
     return StringView();
   while (numLeft() != 0 && std::isdigit(*First))
     ++First;
   return StringView(Tmp, First);
 }
 
 // <positive length number> ::= [0-9]*
while (!consumeIf('E')) {
       Names.push_back(T);
     }
     ExceptionSpec =
       make<DynamicExceptionSpec>(popTrailingNodeArray(SpecsBegin));
     if (!ExceptionSpec)
       return nullptr;
   }
 
-  consumeIf("Dx"); // transaction safe
+  (void)consumeIf("Dx"); // transaction safe
 
   if (!consumeIf('F'))
     return nullptr;
   consumeIf('Y'); // extern "C"
   Node *ReturnType = getDerived().parseType();
   if (ReturnType == nullptr)
     return nullptr;