Re: r350404 - Refactor the way we handle diagnosing unused expression results.
Reverted on the release_80 branch in r353935.
On Fri, Jan 4, 2019 at 6:01 PM Aaron Ballman via cfe-commits
wrote:
>
> Author: aaronballman
> Date: Fri Jan 4 08:58:14 2019
> New Revision: 350404
>
> URL: http://llvm.org/viewvc/llvm-project?rev=350404=rev
> Log:
> Refactor the way we handle diagnosing unused expression results.
>
> Rather than sprinkle calls to DiagnoseUnusedExprResult() around in places
> where we want diagnostics, we now diagnose unused expression statements and
> full expressions in a more generic way when acting on the final expression
> statement. This results in more appropriate diagnostics for [[nodiscard]]
> where we were previously lacking them, such as when the body of a for loop is
> not a compound statement.
>
> This patch fixes PR39837.
>
> Modified:
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Parse/ParseObjc.cpp
> cfe/trunk/lib/Parse/ParseOpenMP.cpp
> cfe/trunk/lib/Parse/ParseStmt.cpp
> cfe/trunk/lib/Sema/SemaCoroutine.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/lib/Sema/SemaLambda.cpp
> cfe/trunk/lib/Sema/SemaOpenMP.cpp
> cfe/trunk/lib/Sema/SemaStmt.cpp
> cfe/trunk/lib/Sema/TreeTransform.h
> cfe/trunk/test/CXX/stmt.stmt/stmt.select/p3.cpp
> cfe/trunk/test/CodeCompletion/pragma-macro-token-caching.c
> cfe/trunk/test/Parser/cxx1z-init-statement.cpp
> cfe/trunk/test/Parser/switch-recovery.cpp
> cfe/trunk/test/SemaCXX/cxx1z-init-statement.cpp
> cfe/trunk/test/SemaCXX/for-range-examples.cpp
> cfe/trunk/test/SemaCXX/warn-unused-result.cpp
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=350404=350403=350404=diff
> ==
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Fri Jan 4 08:58:14 2019
> @@ -360,6 +360,11 @@ class Parser : public CodeCompletionHand
>/// just a regular sub-expression.
>SourceLocation ExprStatementTokLoc;
>
> + /// Tests whether an expression value is discarded based on token
> lookahead.
> + /// It will return true if the lexer is currently processing the })
> + /// terminating a GNU statement expression and false otherwise.
> + bool isExprValueDiscarded();
> +
> public:
>Parser(Preprocessor , Sema , bool SkipFunctionBodies);
>~Parser() override;
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=350404=350403=350404=diff
> ==
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Fri Jan 4 08:58:14 2019
> @@ -1365,6 +1365,7 @@ public:
>void PopCompoundScope();
>
>sema::CompoundScopeInfo () const;
> + bool isCurCompoundStmtAStmtExpr() const;
>
>bool hasAnyUnrecoverableErrorsInThisFunction() const;
>
> @@ -3685,16 +3686,17 @@ public:
> return MakeFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation());
>}
>FullExprArg MakeFullExpr(Expr *Arg, SourceLocation CC) {
> -return FullExprArg(ActOnFinishFullExpr(Arg, CC).get());
> +return FullExprArg(
> +ActOnFinishFullExpr(Arg, CC, /*DiscardedValue*/ false).get());
>}
>FullExprArg MakeFullDiscardedValueExpr(Expr *Arg) {
> ExprResult FE =
> - ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation(),
> - /*DiscardedValue*/ true);
> +ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation(),
> +/*DiscardedValue*/ true);
> return FullExprArg(FE.get());
>}
>
> - StmtResult ActOnExprStmt(ExprResult Arg);
> + StmtResult ActOnExprStmt(ExprResult Arg, bool DiscardedValue = true);
>StmtResult ActOnExprStmtError();
>
>StmtResult ActOnNullStmt(SourceLocation SemiLoc,
> @@ -5340,13 +5342,12 @@ public:
>CreateMaterializeTemporaryExpr(QualType T, Expr *Temporary,
> bool BoundToLvalueReference);
>
> - ExprResult ActOnFinishFullExpr(Expr *Expr) {
> -return ActOnFinishFullExpr(Expr, Expr ? Expr->getExprLoc()
> - : SourceLocation());
> + ExprResult ActOnFinishFullExpr(Expr *Expr, bool DiscardedValue) {
> +return ActOnFinishFullExpr(
> +Expr, Expr ? Expr->getExprLoc() : SourceLocation(), DiscardedValue);
>}
>ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
> - bool DiscardedValue = false,
> - bool IsConstexpr = false);
> + bool DiscardedValue, bool IsConstexpr =
> false);
>
Re: r350404 - Refactor the way we handle diagnosing unused expression results.
On Sun, Jan 6, 2019 at 10:58 AM Nico Weber wrote: > > On Sat, Jan 5, 2019 at 10:16 AM Aaron Ballman wrote: >> >> On Fri, Jan 4, 2019 at 8:41 PM Richard Smith wrote: >> > >> > On Fri, 4 Jan 2019 at 17:33, Nico Weber via cfe-commits >> > wrote: >> >> >> >> Nice, this is finding bugs: >> >> https://bugs.chromium.org/p/chromium/issues/detail?id=919262 >> >> >> >> However, I noticed that for that case, the same warning is printed twice: >> >> >> >> ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): >> >> error: expression result unused [-Werror,-Wunused-value] >> >> NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; >> >>^~~ >> >> ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): >> >> error: expression result unused [-Werror,-Wunused-value] >> >> NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; >> >> >> >> >> >> Is that expected? >> >> Yes and no. It's not unexpected, but it's not super helpful either. As >> Richard points out below, this happens because of template >> instantiation. >> >> > The first diagnostic appears when parsing the template, the second one >> > appears when instantiating it. In the complete diagnostic output: >> > >> > ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): >> > error: expression result unused [-Werror,-Wunused-value] >> > NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; >> >^~~ >> > >> > ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): >> > error: expression result unused [-Werror,-Wunused-value] >> > NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; >> >^~~ >> > ../../third_party/crashpad/crashpad/util/win/process_info.cc(531,17): >> > note: in instantiation of function template specialization >> > 'crashpad::GetProcessBasicInformation' >> > requested here >> > bool result = >> > GetProcessBasicInformation( >> > ^ >> > >> > It'd be nice to suppress the diagnostic during instantiation if it >> > appeared during the initial parse (generally, not with something specific >> > to this warning). >> >> Agreed, though I don't have the time to work on that currently. >> Hopefully the behavior here isn't too onerous for Chromium (or other >> projects)? > > > It wasn't a problem for us, no. The warning found a bug, we fixed the bug, > everyone's happy. Just thought I'd mention the diag looks strange; thanks for > the explanation (and the warning improvement) :-) I'm glad to hear the refactoring is catching more issues, that's great! I'm also glad the template instantiation stuff isn't causing too much of a problem for you. ~Aaron > >> >> >> ~Aaron >> >> > >> >> Code is here: >> >> https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/util/win/process_info.cc?q=crashpad/util/win/process_info.cc=package:chromium=0=227 >> >> https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/util/win/ntstatus_logging.h?type=cs=NTSTATUS_LOG=package:chromium=0=63 >> >> >> >> >> >> On Fri, Jan 4, 2019 at 12:01 PM Aaron Ballman via cfe-commits >> >> wrote: >> >>> >> >>> Author: aaronballman >> >>> Date: Fri Jan 4 08:58:14 2019 >> >>> New Revision: 350404 >> >>> >> >>> URL: http://llvm.org/viewvc/llvm-project?rev=350404=rev >> >>> Log: >> >>> Refactor the way we handle diagnosing unused expression results. >> >>> >> >>> Rather than sprinkle calls to DiagnoseUnusedExprResult() around in >> >>> places where we want diagnostics, we now diagnose unused expression >> >>> statements and full expressions in a more generic way when acting on the >> >>> final expression statement. This results in more appropriate diagnostics >> >>> for [[nodiscard]] where we were previously lacking them, such as when >> >>> the body of a for loop is not a compound statement. >> >>> >> >>> This patch fixes PR39837. >> >>> >> >>> Modified: >> >>> cfe/trunk/include/clang/Parse/Parser.h >> >>> cfe/trunk/include/clang/Sema/Sema.h >> >>> cfe/trunk/lib/Parse/ParseObjc.cpp >> >>> cfe/trunk/lib/Parse/ParseOpenMP.cpp >> >>> cfe/trunk/lib/Parse/ParseStmt.cpp >> >>> cfe/trunk/lib/Sema/SemaCoroutine.cpp >> >>> cfe/trunk/lib/Sema/SemaDecl.cpp >> >>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp >> >>> cfe/trunk/lib/Sema/SemaExpr.cpp >> >>> cfe/trunk/lib/Sema/SemaExprCXX.cpp >> >>> cfe/trunk/lib/Sema/SemaLambda.cpp >> >>> cfe/trunk/lib/Sema/SemaOpenMP.cpp >> >>> cfe/trunk/lib/Sema/SemaStmt.cpp >> >>> cfe/trunk/lib/Sema/TreeTransform.h >> >>> cfe/trunk/test/CXX/stmt.stmt/stmt.select/p3.cpp >> >>> cfe/trunk/test/CodeCompletion/pragma-macro-token-caching.c >> >>> cfe/trunk/test/Parser/cxx1z-init-statement.cpp >> >>> cfe/trunk/test/Parser/switch-recovery.cpp >> >>>
Re: r350404 - Refactor the way we handle diagnosing unused expression results.
On Fri, Jan 4, 2019 at 8:41 PM Richard Smith wrote: > > On Fri, 4 Jan 2019 at 17:33, Nico Weber via cfe-commits > wrote: >> >> Nice, this is finding bugs: >> https://bugs.chromium.org/p/chromium/issues/detail?id=919262 >> >> However, I noticed that for that case, the same warning is printed twice: >> >> ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): >> error: expression result unused [-Werror,-Wunused-value] >> NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; >>^~~ >> ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): >> error: expression result unused [-Werror,-Wunused-value] >> NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; >> >> >> Is that expected? Yes and no. It's not unexpected, but it's not super helpful either. As Richard points out below, this happens because of template instantiation. > The first diagnostic appears when parsing the template, the second one > appears when instantiating it. In the complete diagnostic output: > > ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): error: > expression result unused [-Werror,-Wunused-value] > NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; >^~~ > > ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): error: > expression result unused [-Werror,-Wunused-value] > NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; >^~~ > ../../third_party/crashpad/crashpad/util/win/process_info.cc(531,17): note: > in instantiation of function template specialization > 'crashpad::GetProcessBasicInformation' > requested here > bool result = GetProcessBasicInformation( > ^ > > It'd be nice to suppress the diagnostic during instantiation if it appeared > during the initial parse (generally, not with something specific to this > warning). Agreed, though I don't have the time to work on that currently. Hopefully the behavior here isn't too onerous for Chromium (or other projects)? ~Aaron > >> Code is here: >> https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/util/win/process_info.cc?q=crashpad/util/win/process_info.cc=package:chromium=0=227 >> https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/util/win/ntstatus_logging.h?type=cs=NTSTATUS_LOG=package:chromium=0=63 >> >> >> On Fri, Jan 4, 2019 at 12:01 PM Aaron Ballman via cfe-commits >> wrote: >>> >>> Author: aaronballman >>> Date: Fri Jan 4 08:58:14 2019 >>> New Revision: 350404 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=350404=rev >>> Log: >>> Refactor the way we handle diagnosing unused expression results. >>> >>> Rather than sprinkle calls to DiagnoseUnusedExprResult() around in places >>> where we want diagnostics, we now diagnose unused expression statements and >>> full expressions in a more generic way when acting on the final expression >>> statement. This results in more appropriate diagnostics for [[nodiscard]] >>> where we were previously lacking them, such as when the body of a for loop >>> is not a compound statement. >>> >>> This patch fixes PR39837. >>> >>> Modified: >>> cfe/trunk/include/clang/Parse/Parser.h >>> cfe/trunk/include/clang/Sema/Sema.h >>> cfe/trunk/lib/Parse/ParseObjc.cpp >>> cfe/trunk/lib/Parse/ParseOpenMP.cpp >>> cfe/trunk/lib/Parse/ParseStmt.cpp >>> cfe/trunk/lib/Sema/SemaCoroutine.cpp >>> cfe/trunk/lib/Sema/SemaDecl.cpp >>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp >>> cfe/trunk/lib/Sema/SemaExpr.cpp >>> cfe/trunk/lib/Sema/SemaExprCXX.cpp >>> cfe/trunk/lib/Sema/SemaLambda.cpp >>> cfe/trunk/lib/Sema/SemaOpenMP.cpp >>> cfe/trunk/lib/Sema/SemaStmt.cpp >>> cfe/trunk/lib/Sema/TreeTransform.h >>> cfe/trunk/test/CXX/stmt.stmt/stmt.select/p3.cpp >>> cfe/trunk/test/CodeCompletion/pragma-macro-token-caching.c >>> cfe/trunk/test/Parser/cxx1z-init-statement.cpp >>> cfe/trunk/test/Parser/switch-recovery.cpp >>> cfe/trunk/test/SemaCXX/cxx1z-init-statement.cpp >>> cfe/trunk/test/SemaCXX/for-range-examples.cpp >>> cfe/trunk/test/SemaCXX/warn-unused-result.cpp >>> >>> Modified: cfe/trunk/include/clang/Parse/Parser.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=350404=350403=350404=diff >>> == >>> --- cfe/trunk/include/clang/Parse/Parser.h (original) >>> +++ cfe/trunk/include/clang/Parse/Parser.h Fri Jan 4 08:58:14 2019 >>> @@ -360,6 +360,11 @@ class Parser : public CodeCompletionHand >>>/// just a regular sub-expression. >>>SourceLocation ExprStatementTokLoc; >>> >>> + /// Tests whether an expression value is discarded based on token >>> lookahead. >>> + /// It will return true if the lexer is
Re: r350404 - Refactor the way we handle diagnosing unused expression results.
On Fri, 4 Jan 2019 at 17:33, Nico Weber via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Nice, this is finding bugs: > https://bugs.chromium.org/p/chromium/issues/detail?id=919262 > > However, I noticed that for that case, the same warning is printed twice: > > ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): > error: expression result unused [-Werror,-Wunused-value] > NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; >^~~ > ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): > error: expression result unused [-Werror,-Wunused-value] > NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; > > > Is that expected? > The first diagnostic appears when parsing the template, the second one appears when instantiating it. In the complete diagnostic output: ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): error: expression result unused [-Werror,-Wunused-value] NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; ^~~ ../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36): error: expression result unused [-Werror,-Wunused-value] NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; ^~~ ../../third_party/crashpad/crashpad/util/win/process_info.cc(531,17): note: in instantiation of function template specialization 'crashpad::GetProcessBasicInformation' requested here bool result = GetProcessBasicInformation( ^ It'd be nice to suppress the diagnostic during instantiation if it appeared during the initial parse (generally, not with something specific to this warning). Code is here: > > https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/util/win/process_info.cc?q=crashpad/util/win/process_info.cc=package:chromium=0=227 > > https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/util/win/ntstatus_logging.h?type=cs=NTSTATUS_LOG=package:chromium=0=63 > > > On Fri, Jan 4, 2019 at 12:01 PM Aaron Ballman via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: aaronballman >> Date: Fri Jan 4 08:58:14 2019 >> New Revision: 350404 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=350404=rev >> Log: >> Refactor the way we handle diagnosing unused expression results. >> >> Rather than sprinkle calls to DiagnoseUnusedExprResult() around in places >> where we want diagnostics, we now diagnose unused expression statements and >> full expressions in a more generic way when acting on the final expression >> statement. This results in more appropriate diagnostics for [[nodiscard]] >> where we were previously lacking them, such as when the body of a for loop >> is not a compound statement. >> >> This patch fixes PR39837. >> >> Modified: >> cfe/trunk/include/clang/Parse/Parser.h >> cfe/trunk/include/clang/Sema/Sema.h >> cfe/trunk/lib/Parse/ParseObjc.cpp >> cfe/trunk/lib/Parse/ParseOpenMP.cpp >> cfe/trunk/lib/Parse/ParseStmt.cpp >> cfe/trunk/lib/Sema/SemaCoroutine.cpp >> cfe/trunk/lib/Sema/SemaDecl.cpp >> cfe/trunk/lib/Sema/SemaDeclCXX.cpp >> cfe/trunk/lib/Sema/SemaExpr.cpp >> cfe/trunk/lib/Sema/SemaExprCXX.cpp >> cfe/trunk/lib/Sema/SemaLambda.cpp >> cfe/trunk/lib/Sema/SemaOpenMP.cpp >> cfe/trunk/lib/Sema/SemaStmt.cpp >> cfe/trunk/lib/Sema/TreeTransform.h >> cfe/trunk/test/CXX/stmt.stmt/stmt.select/p3.cpp >> cfe/trunk/test/CodeCompletion/pragma-macro-token-caching.c >> cfe/trunk/test/Parser/cxx1z-init-statement.cpp >> cfe/trunk/test/Parser/switch-recovery.cpp >> cfe/trunk/test/SemaCXX/cxx1z-init-statement.cpp >> cfe/trunk/test/SemaCXX/for-range-examples.cpp >> cfe/trunk/test/SemaCXX/warn-unused-result.cpp >> >> Modified: cfe/trunk/include/clang/Parse/Parser.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=350404=350403=350404=diff >> >> == >> --- cfe/trunk/include/clang/Parse/Parser.h (original) >> +++ cfe/trunk/include/clang/Parse/Parser.h Fri Jan 4 08:58:14 2019 >> @@ -360,6 +360,11 @@ class Parser : public CodeCompletionHand >>/// just a regular sub-expression. >>SourceLocation ExprStatementTokLoc; >> >> + /// Tests whether an expression value is discarded based on token >> lookahead. >> + /// It will return true if the lexer is currently processing the }) >> + /// terminating a GNU statement expression and false otherwise. >> + bool isExprValueDiscarded(); >> + >> public: >>Parser(Preprocessor , Sema , bool SkipFunctionBodies); >>~Parser() override; >> >> Modified: cfe/trunk/include/clang/Sema/Sema.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=350404=350403=350404=diff >> >>
Re: r350404 - Refactor the way we handle diagnosing unused expression results.
Nice, this is finding bugs:
https://bugs.chromium.org/p/chromium/issues/detail?id=919262
However, I noticed that for that case, the same warning is printed twice:
../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36):
error: expression result unused [-Werror,-Wunused-value]
NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess";
^~~
../../third_party/crashpad/crashpad/util/win/process_info.cc(227,36):
error: expression result unused [-Werror,-Wunused-value]
NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess";
Is that expected? Code is here:
https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/util/win/process_info.cc?q=crashpad/util/win/process_info.cc=package:chromium=0=227
https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/util/win/ntstatus_logging.h?type=cs=NTSTATUS_LOG=package:chromium=0=63
On Fri, Jan 4, 2019 at 12:01 PM Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: aaronballman
> Date: Fri Jan 4 08:58:14 2019
> New Revision: 350404
>
> URL: http://llvm.org/viewvc/llvm-project?rev=350404=rev
> Log:
> Refactor the way we handle diagnosing unused expression results.
>
> Rather than sprinkle calls to DiagnoseUnusedExprResult() around in places
> where we want diagnostics, we now diagnose unused expression statements and
> full expressions in a more generic way when acting on the final expression
> statement. This results in more appropriate diagnostics for [[nodiscard]]
> where we were previously lacking them, such as when the body of a for loop
> is not a compound statement.
>
> This patch fixes PR39837.
>
> Modified:
> cfe/trunk/include/clang/Parse/Parser.h
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Parse/ParseObjc.cpp
> cfe/trunk/lib/Parse/ParseOpenMP.cpp
> cfe/trunk/lib/Parse/ParseStmt.cpp
> cfe/trunk/lib/Sema/SemaCoroutine.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/lib/Sema/SemaLambda.cpp
> cfe/trunk/lib/Sema/SemaOpenMP.cpp
> cfe/trunk/lib/Sema/SemaStmt.cpp
> cfe/trunk/lib/Sema/TreeTransform.h
> cfe/trunk/test/CXX/stmt.stmt/stmt.select/p3.cpp
> cfe/trunk/test/CodeCompletion/pragma-macro-token-caching.c
> cfe/trunk/test/Parser/cxx1z-init-statement.cpp
> cfe/trunk/test/Parser/switch-recovery.cpp
> cfe/trunk/test/SemaCXX/cxx1z-init-statement.cpp
> cfe/trunk/test/SemaCXX/for-range-examples.cpp
> cfe/trunk/test/SemaCXX/warn-unused-result.cpp
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=350404=350403=350404=diff
>
> ==
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Fri Jan 4 08:58:14 2019
> @@ -360,6 +360,11 @@ class Parser : public CodeCompletionHand
>/// just a regular sub-expression.
>SourceLocation ExprStatementTokLoc;
>
> + /// Tests whether an expression value is discarded based on token
> lookahead.
> + /// It will return true if the lexer is currently processing the })
> + /// terminating a GNU statement expression and false otherwise.
> + bool isExprValueDiscarded();
> +
> public:
>Parser(Preprocessor , Sema , bool SkipFunctionBodies);
>~Parser() override;
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=350404=350403=350404=diff
>
> ==
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Fri Jan 4 08:58:14 2019
> @@ -1365,6 +1365,7 @@ public:
>void PopCompoundScope();
>
>sema::CompoundScopeInfo () const;
> + bool isCurCompoundStmtAStmtExpr() const;
>
>bool hasAnyUnrecoverableErrorsInThisFunction() const;
>
> @@ -3685,16 +3686,17 @@ public:
> return MakeFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation());
>}
>FullExprArg MakeFullExpr(Expr *Arg, SourceLocation CC) {
> -return FullExprArg(ActOnFinishFullExpr(Arg, CC).get());
> +return FullExprArg(
> +ActOnFinishFullExpr(Arg, CC, /*DiscardedValue*/ false).get());
>}
>FullExprArg MakeFullDiscardedValueExpr(Expr *Arg) {
> ExprResult FE =
> - ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation(),
> - /*DiscardedValue*/ true);
> +ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() :
> SourceLocation(),
> +/*DiscardedValue*/ true);
> return FullExprArg(FE.get());
>}
>
> - StmtResult ActOnExprStmt(ExprResult Arg);
> + StmtResult ActOnExprStmt(ExprResult Arg, bool DiscardedValue = true);
>
r350404 - Refactor the way we handle diagnosing unused expression results.
Author: aaronballman
Date: Fri Jan 4 08:58:14 2019
New Revision: 350404
URL: http://llvm.org/viewvc/llvm-project?rev=350404=rev
Log:
Refactor the way we handle diagnosing unused expression results.
Rather than sprinkle calls to DiagnoseUnusedExprResult() around in places where
we want diagnostics, we now diagnose unused expression statements and full
expressions in a more generic way when acting on the final expression
statement. This results in more appropriate diagnostics for [[nodiscard]] where
we were previously lacking them, such as when the body of a for loop is not a
compound statement.
This patch fixes PR39837.
Modified:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseObjc.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaLambda.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/CXX/stmt.stmt/stmt.select/p3.cpp
cfe/trunk/test/CodeCompletion/pragma-macro-token-caching.c
cfe/trunk/test/Parser/cxx1z-init-statement.cpp
cfe/trunk/test/Parser/switch-recovery.cpp
cfe/trunk/test/SemaCXX/cxx1z-init-statement.cpp
cfe/trunk/test/SemaCXX/for-range-examples.cpp
cfe/trunk/test/SemaCXX/warn-unused-result.cpp
Modified: cfe/trunk/include/clang/Parse/Parser.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=350404=350403=350404=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Fri Jan 4 08:58:14 2019
@@ -360,6 +360,11 @@ class Parser : public CodeCompletionHand
/// just a regular sub-expression.
SourceLocation ExprStatementTokLoc;
+ /// Tests whether an expression value is discarded based on token lookahead.
+ /// It will return true if the lexer is currently processing the })
+ /// terminating a GNU statement expression and false otherwise.
+ bool isExprValueDiscarded();
+
public:
Parser(Preprocessor , Sema , bool SkipFunctionBodies);
~Parser() override;
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=350404=350403=350404=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Jan 4 08:58:14 2019
@@ -1365,6 +1365,7 @@ public:
void PopCompoundScope();
sema::CompoundScopeInfo () const;
+ bool isCurCompoundStmtAStmtExpr() const;
bool hasAnyUnrecoverableErrorsInThisFunction() const;
@@ -3685,16 +3686,17 @@ public:
return MakeFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation());
}
FullExprArg MakeFullExpr(Expr *Arg, SourceLocation CC) {
-return FullExprArg(ActOnFinishFullExpr(Arg, CC).get());
+return FullExprArg(
+ActOnFinishFullExpr(Arg, CC, /*DiscardedValue*/ false).get());
}
FullExprArg MakeFullDiscardedValueExpr(Expr *Arg) {
ExprResult FE =
- ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation(),
- /*DiscardedValue*/ true);
+ActOnFinishFullExpr(Arg, Arg ? Arg->getExprLoc() : SourceLocation(),
+/*DiscardedValue*/ true);
return FullExprArg(FE.get());
}
- StmtResult ActOnExprStmt(ExprResult Arg);
+ StmtResult ActOnExprStmt(ExprResult Arg, bool DiscardedValue = true);
StmtResult ActOnExprStmtError();
StmtResult ActOnNullStmt(SourceLocation SemiLoc,
@@ -5340,13 +5342,12 @@ public:
CreateMaterializeTemporaryExpr(QualType T, Expr *Temporary,
bool BoundToLvalueReference);
- ExprResult ActOnFinishFullExpr(Expr *Expr) {
-return ActOnFinishFullExpr(Expr, Expr ? Expr->getExprLoc()
- : SourceLocation());
+ ExprResult ActOnFinishFullExpr(Expr *Expr, bool DiscardedValue) {
+return ActOnFinishFullExpr(
+Expr, Expr ? Expr->getExprLoc() : SourceLocation(), DiscardedValue);
}
ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
- bool DiscardedValue = false,
- bool IsConstexpr = false);
+ bool DiscardedValue, bool IsConstexpr =
false);
StmtResult ActOnFinishFullStmt(Stmt *Stmt);
// Marks SS invalid if it represents an incomplete type.
Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=350404=350403=350404=diff
