[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17
lebedev.ri updated this revision to Diff 152587.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
Herald added a subscriber: llvm-commits.
Right, sorry. Let's try to revive this.
Does this test coverage look better?
What did i miss?
Repository:
rL LLVM
https://reviews.llvm.org/D45179
Files:
include/__config
test/libcxx/diagnostics/disable_nodiscard_aftercxx17.fail.cpp
test/libcxx/diagnostics/disable_nodiscard_aftercxx17.pass.cpp
test/libcxx/diagnostics/force_nodiscard.fail.cpp
test/libcxx/diagnostics/force_nodiscard_disabled.fail.cpp
test/libcxx/diagnostics/force_nodiscard_disabled.pass.cpp
test/libcxx/diagnostics/nodiscard.fail.cpp
test/libcxx/diagnostics/nodiscard.pass.cpp
test/libcxx/diagnostics/nodiscard_aftercxx17.fail.cpp
Index: test/libcxx/diagnostics/nodiscard_aftercxx17.fail.cpp
===
--- test/libcxx/diagnostics/nodiscard_aftercxx17.fail.cpp
+++ test/libcxx/diagnostics/nodiscard_aftercxx17.fail.cpp
@@ -16,9 +16,10 @@
#include <__config>
-_LIBCPP_NODISCARD_AFTER_CXX17 int foo() { return 6; }
+_LIBCPP_NODISCARD int foo() { return 6; }
+_LIBCPP_NODISCARD_AFTER_CXX17 int bar() { return 6; }
-int main ()
-{
- foo(); // expected-error {{ignoring return value of function declared with 'nodiscard' attribute}}
+int main() {
+ foo(); // expected-error {{ignoring return value of function declared with 'nodiscard' attribute}}
+ bar(); // expected-error {{ignoring return value of function declared with 'nodiscard' attribute}}
}
Index: test/libcxx/diagnostics/force_nodiscard_disabled.pass.cpp
===
--- /dev/null
+++ test/libcxx/diagnostics/force_nodiscard_disabled.pass.cpp
@@ -0,0 +1,24 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// Test that _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17 overrides
+// _LIBCPP_FORCE_NODISCARD define, always.
+
+// MODULES_DEFINES: _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+// MODULES_DEFINES: _LIBCPP_FORCE_NODISCARD
+#define _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+#define _LIBCPP_FORCE_NODISCARD
+#include <__config>
+
+_LIBCPP_NODISCARD_AFTER_CXX17 int bar() { return 7; }
+
+int main() {
+ bar(); // no error here!
+}
Index: test/libcxx/diagnostics/force_nodiscard_disabled.fail.cpp
===
--- /dev/null
+++ test/libcxx/diagnostics/force_nodiscard_disabled.fail.cpp
@@ -0,0 +1,27 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// Test that _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17 overrides
+// _LIBCPP_NODISCARD_AFTER_CXX17 define, always.
+// But not the _LIBCPP_NODISCARD.
+
+// MODULES_DEFINES: _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+// MODULES_DEFINES: _LIBCPP_FORCE_NODISCARD
+#define _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+#define _LIBCPP_FORCE_NODISCARD
+#include <__config>
+
+_LIBCPP_NODISCARD int foo() { return 6; }
+_LIBCPP_NODISCARD_AFTER_CXX17 int bar() { return 7; }
+
+int main() {
+ foo(); // expected-error {{ignoring return value of function declared with}}
+ bar(); // no error here!
+}
Index: test/libcxx/diagnostics/force_nodiscard.fail.cpp
===
--- /dev/null
+++ test/libcxx/diagnostics/force_nodiscard.fail.cpp
@@ -0,0 +1,30 @@
+// -*- C++ -*-
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// Test that _LIBCPP_FORCE_NODISCARD always enables nodiscard, regardless of
+// the standard version.
+
+// REQUIRES: clang || apple-clang
+
+// This won't work in gcc before c++17.
+
+// MODULES_DEFINES: _LIBCPP_FORCE_NODISCARD
+#define _LIBCPP_FORCE_NODISCARD
+#include <__config>
+
+_LIBCPP_NODISCARD int foo() { return 6; }
+_LIBCPP_NODISCARD_AFTER_CXX17 int bar() { return 7; }
+
+int main() {
+ foo(); // expected-error {{ignoring return value of function declared with}}
+ bar(); // expected-error {{ignoring return value of function declared with}}
+ // The actual attribute used may be different, so it should n
[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations
modocache created this revision.
modocache added reviewers: GorNishanov, EricWF.
Add citations to the Coroutines TS to the `isValidCoroutineContext`
function, as well as a FIXME and test for [expr.await]p2, which states
a co_await expression cannot be used in a default argument.
Test Plan: check-clang
Repository:
rC Clang
https://reviews.llvm.org/D48519
Files:
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -1,3 +1,6 @@
+// This file contains references to sections of the Coroutines TS, which can be
+// found at http://wg21.link/coroutines.
+
// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -verify %s -fcxx-exceptions
-fexceptions -Wunused-result
void no_coroutine_traits_bad_arg_await() {
@@ -320,6 +323,11 @@
typeid(co_yield a); // expected-error {{cannot be used in an unevaluated
context}}
}
+// [expr.await]p2: "An await-expression shall not appear in a default
argument."
+// FIXME: A better diagnostic would explicitly state that default arguments are
+// not allowed. A user may not understand that this is "outside a function."
+void default_argument(int arg = co_await 0) {} // expected-error {{'co_await'
cannot be used outside a function}}
+
constexpr auto constexpr_deduced_return_coroutine() {
co_yield 0; // expected-error {{'co_yield' cannot be used in a constexpr
function}}
// expected-error@-1 {{'co_yield' cannot be used in a function with a
deduced return type}}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -9,6 +9,9 @@
//
// This file implements semantic analysis for C++ Coroutines.
//
+// This file contains references to sections of the Coroutines TS, which
+// can be found at http://wg21.link/coroutines.
+//
//===--===//
#include "CoroutineStmtBuilder.h"
@@ -196,13 +199,25 @@
static bool isValidCoroutineContext(Sema &S, SourceLocation Loc,
StringRef Keyword) {
- // 'co_await' and 'co_yield' are not permitted in unevaluated operands.
+ // 'co_await' and 'co_yield' are not permitted in unevaluated operands,
+ // such as subexpressions of \c sizeof.
+ //
+ // [expr.await]p2, emphasis added: "An await-expression shall appear only in
+ // a *potentially evaluated* expression within the compound-statement of a
+ // function-body outside of a handler [...] A context within a function where
+ // an await-expression can appear is called a suspension context of the
+ // function." And per [expr.yield]p1: "A yield-expression shall appear only
+ // within a suspension context of a function."
if (S.isUnevaluatedContext()) {
S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
return false;
}
- // Any other usage must be within a function.
+ // Per [expr.await]p2, any other usage must be within a function.
+ // FIXME: This also covers [expr.await]p2: "An await-expression shall not
+ // appear in a default argument." But the diagnostic QoI here could be
+ // improved to inform the user that default arguments specifically are not
+ // allowed.
auto *FD = dyn_cast(S.CurContext);
if (!FD) {
S.Diag(Loc, isa(S.CurContext)
@@ -233,22 +248,33 @@
// Diagnose when a constructor, destructor, copy/move assignment operator,
// or the function 'main' are declared as a coroutine.
auto *MD = dyn_cast(FD);
+ // [class.ctor]p6: "A constructor shall not be a coroutine."
if (MD && isa(MD))
return DiagInvalid(DiagCtor);
+ // [class.dtor]p17: "A destructor shall not be a coroutine."
else if (MD && isa(MD))
return DiagInvalid(DiagDtor);
else if (MD && MD->isCopyAssignmentOperator())
return DiagInvalid(DiagCopyAssign);
else if (MD && MD->isMoveAssignmentOperator())
return DiagInvalid(DiagMoveAssign);
+ // [basic.start.main]p3: "The function main shall not be a coroutine."
else if (FD->isMain())
return DiagInvalid(DiagMain);
// Emit a diagnostics for each of the following conditions which is not met.
+ // [expr.const]p2: "An expression e is a core constant expression unless the
+ // evaluation of e [...] would evaluate one of the following expressions:
+ // [...] an await-expression [...] a yield-expression."
if (FD->isConstexpr())
DiagInvalid(DiagConstexpr);
+ // [dcl.spec.auto]p15: "A function declared with a return type that uses a
+ // placeholder type shall not be a coroutine."
if (FD->getReturnType()->isUndeducedType())
DiagInvalid(DiagAutoRet);
+ // [dcl.fct.def.coroutine]p1: "The parameter-declaration-clause of the
+ // coroutine shall not terminate with an ellipsis that is not part of a
+ // parameter-declaration."
if (FD->isVariadic())
Dia
[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations
modocache added a comment. Also @GorNishanov I'm curious about your two cents on whether comments like these are valuable. If you think they are I may add a few more with post-commit review. Repository: rC Clang https://reviews.llvm.org/D48519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations
modocache added inline comments. Comment at: lib/Sema/SemaCoroutine.cpp:260 else if (MD && MD->isMoveAssignmentOperator()) return DiagInvalid(DiagMoveAssign); + // [basic.start.main]p3: "The function main shall not be a coroutine." @GorNishanov Is there anything in the TS that states copy and move assignment operators shall not include await or yield expressions? These were added D25292 but I'm not sure whether I'm missing something in the TS text, or if maybe this language was in a prior revision of the TS. Repository: rC Clang https://reviews.llvm.org/D48519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47673: [Coroutines] Less IR for noexcept await_resume
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM with some suggestions. Comment at: lib/CodeGen/CGCoroutine.cpp:224 + bool ResumeCanThrow = true; + if (const auto *MCE = dyn_cast(S.getResumeExpr())) +if (const auto *Proto = This long sequence of if statements seems to be asking to be put into its own predicate function: expressionCanThrow or something like that. Repository: rC Clang https://reviews.llvm.org/D47673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations
GorNishanov accepted this revision. GorNishanov added a subscriber: rsmith. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Sema/SemaCoroutine.cpp:260 else if (MD && MD->isMoveAssignmentOperator()) return DiagInvalid(DiagMoveAssign); + // [basic.start.main]p3: "The function main shall not be a coroutine." modocache wrote: > @GorNishanov Is there anything in the TS that states copy and move assignment > operators shall not include await or yield expressions? These were added > D25292 but I'm not sure whether I'm missing something in the TS text, or if > maybe this language was in a prior revision of the TS. Yes. N4499/[special] said: A special member function shall not be a coroutine. I think @rsmith wanted to relax it, but, I am not sure if he had a use case in mind. I am thinking putting the restriction from N4499 back. My approach is if in doubt, be more restrictive initially, then, we can relax if use cases are discovered. It will be a non-breaking change. Repository: rC Clang https://reviews.llvm.org/D48519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations
modocache updated this revision to Diff 152597.
modocache added a comment.
Great, thanks for the review! I added a reference to N4499.
Repository:
rC Clang
https://reviews.llvm.org/D48519
Files:
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -1,3 +1,6 @@
+// This file contains references to sections of the Coroutines TS, which can be
+// found at http://wg21.link/coroutines.
+
// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -Wunused-result
void no_coroutine_traits_bad_arg_await() {
@@ -320,6 +323,11 @@
typeid(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
}
+// [expr.await]p2: "An await-expression shall not appear in a default argument."
+// FIXME: A better diagnostic would explicitly state that default arguments are
+// not allowed. A user may not understand that this is "outside a function."
+void default_argument(int arg = co_await 0) {} // expected-error {{'co_await' cannot be used outside a function}}
+
constexpr auto constexpr_deduced_return_coroutine() {
co_yield 0; // expected-error {{'co_yield' cannot be used in a constexpr function}}
// expected-error@-1 {{'co_yield' cannot be used in a function with a deduced return type}}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -9,6 +9,9 @@
//
// This file implements semantic analysis for C++ Coroutines.
//
+// This file contains references to sections of the Coroutines TS, which
+// can be found at http://wg21.link/coroutines.
+//
//===--===//
#include "CoroutineStmtBuilder.h"
@@ -196,13 +199,25 @@
static bool isValidCoroutineContext(Sema &S, SourceLocation Loc,
StringRef Keyword) {
- // 'co_await' and 'co_yield' are not permitted in unevaluated operands.
+ // 'co_await' and 'co_yield' are not permitted in unevaluated operands,
+ // such as subexpressions of \c sizeof.
+ //
+ // [expr.await]p2, emphasis added: "An await-expression shall appear only in
+ // a *potentially evaluated* expression within the compound-statement of a
+ // function-body outside of a handler [...] A context within a function where
+ // an await-expression can appear is called a suspension context of the
+ // function." And per [expr.yield]p1: "A yield-expression shall appear only
+ // within a suspension context of a function."
if (S.isUnevaluatedContext()) {
S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
return false;
}
- // Any other usage must be within a function.
+ // Per [expr.await]p2, any other usage must be within a function.
+ // FIXME: This also covers [expr.await]p2: "An await-expression shall not
+ // appear in a default argument." But the diagnostic QoI here could be
+ // improved to inform the user that default arguments specifically are not
+ // allowed.
auto *FD = dyn_cast(S.CurContext);
if (!FD) {
S.Diag(Loc, isa(S.CurContext)
@@ -233,22 +248,37 @@
// Diagnose when a constructor, destructor, copy/move assignment operator,
// or the function 'main' are declared as a coroutine.
auto *MD = dyn_cast(FD);
+ // [class.ctor]p6: "A constructor shall not be a coroutine."
if (MD && isa(MD))
return DiagInvalid(DiagCtor);
+ // [class.dtor]p17: "A destructor shall not be a coroutine."
else if (MD && isa(MD))
return DiagInvalid(DiagDtor);
+ // N4499 [special]p6: "A special member function shall not be a coroutine."
+ // Per C++ [special]p1, special member functions are the "default constructor,
+ // copy constructor and copy assignment operator, move constructor and move
+ // assignment operator, and destructor."
else if (MD && MD->isCopyAssignmentOperator())
return DiagInvalid(DiagCopyAssign);
else if (MD && MD->isMoveAssignmentOperator())
return DiagInvalid(DiagMoveAssign);
+ // [basic.start.main]p3: "The function main shall not be a coroutine."
else if (FD->isMain())
return DiagInvalid(DiagMain);
// Emit a diagnostics for each of the following conditions which is not met.
+ // [expr.const]p2: "An expression e is a core constant expression unless the
+ // evaluation of e [...] would evaluate one of the following expressions:
+ // [...] an await-expression [...] a yield-expression."
if (FD->isConstexpr())
DiagInvalid(DiagConstexpr);
+ // [dcl.spec.auto]p15: "A function declared with a return type that uses a
+ // placeholder type shall not be a coroutine."
if (FD->getReturnType()->isUndeducedType())
DiagInvalid(DiagAutoRet);
+ // [dcl.fct.def.coroutine]p1: "The parameter-declaration-clause of the
+ // coroutine shall not termi
r335419 - Attempt to fix latent tablegen dependency issue
Author: rnk
Date: Sat Jun 23 10:32:51 2018
New Revision: 335419
URL: http://llvm.org/viewvc/llvm-project?rev=335419&view=rev
Log:
Attempt to fix latent tablegen dependency issue
Modified:
cfe/trunk/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
Modified: cfe/trunk/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-fuzzer/handle-llvm/CMakeLists.txt?rev=335419&r1=335418&r2=335419&view=diff
==
--- cfe/trunk/tools/clang-fuzzer/handle-llvm/CMakeLists.txt (original)
+++ cfe/trunk/tools/clang-fuzzer/handle-llvm/CMakeLists.txt Sat Jun 23 10:32:51
2018
@@ -4,8 +4,17 @@ set(LLVM_LINK_COMPONENTS
MC
Support
Analysis
-)
+ )
+
+# Depend on LLVM IR intrinsic generation.
+set(handle_llvm_deps intrinsics_gen)
+if (CLANG_BUILT_STANDALONE)
+ set(handle_llvm_deps)
+endif()
add_clang_library(clangHandleLLVM
handle_llvm.cpp
+
+ DEPENDS
+ ${handle_llvm_deps}
)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335420 - [Sema] isValidCoroutineContext FIXME and citations
Author: modocache
Date: Sat Jun 23 11:01:02 2018
New Revision: 335420
URL: http://llvm.org/viewvc/llvm-project?rev=335420&view=rev
Log:
[Sema] isValidCoroutineContext FIXME and citations
Summary:
Add citations to the Coroutines TS to the `isValidCoroutineContext`
function, as well as a FIXME and test for [expr.await]p2, which states
a co_await expression cannot be used in a default argument.
Test Plan: check-clang
Reviewers: GorNishanov, EricWF
Reviewed By: GorNishanov
Subscribers: rsmith, cfe-commits
Differential Revision: https://reviews.llvm.org/D48519
Modified:
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp
Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=335420&r1=335419&r2=335420&view=diff
==
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Sat Jun 23 11:01:02 2018
@@ -9,6 +9,9 @@
//
// This file implements semantic analysis for C++ Coroutines.
//
+// This file contains references to sections of the Coroutines TS, which
+// can be found at http://wg21.link/coroutines.
+//
//===--===//
#include "CoroutineStmtBuilder.h"
@@ -196,13 +199,25 @@ static QualType lookupCoroutineHandleTyp
static bool isValidCoroutineContext(Sema &S, SourceLocation Loc,
StringRef Keyword) {
- // 'co_await' and 'co_yield' are not permitted in unevaluated operands.
+ // 'co_await' and 'co_yield' are not permitted in unevaluated operands,
+ // such as subexpressions of \c sizeof.
+ //
+ // [expr.await]p2, emphasis added: "An await-expression shall appear only in
+ // a *potentially evaluated* expression within the compound-statement of a
+ // function-body outside of a handler [...] A context within a function where
+ // an await-expression can appear is called a suspension context of the
+ // function." And per [expr.yield]p1: "A yield-expression shall appear only
+ // within a suspension context of a function."
if (S.isUnevaluatedContext()) {
S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
return false;
}
- // Any other usage must be within a function.
+ // Per [expr.await]p2, any other usage must be within a function.
+ // FIXME: This also covers [expr.await]p2: "An await-expression shall not
+ // appear in a default argument." But the diagnostic QoI here could be
+ // improved to inform the user that default arguments specifically are not
+ // allowed.
auto *FD = dyn_cast(S.CurContext);
if (!FD) {
S.Diag(Loc, isa(S.CurContext)
@@ -233,22 +248,37 @@ static bool isValidCoroutineContext(Sema
// Diagnose when a constructor, destructor, copy/move assignment operator,
// or the function 'main' are declared as a coroutine.
auto *MD = dyn_cast(FD);
+ // [class.ctor]p6: "A constructor shall not be a coroutine."
if (MD && isa(MD))
return DiagInvalid(DiagCtor);
+ // [class.dtor]p17: "A destructor shall not be a coroutine."
else if (MD && isa(MD))
return DiagInvalid(DiagDtor);
+ // N4499 [special]p6: "A special member function shall not be a coroutine."
+ // Per C++ [special]p1, special member functions are the "default
constructor,
+ // copy constructor and copy assignment operator, move constructor and move
+ // assignment operator, and destructor."
else if (MD && MD->isCopyAssignmentOperator())
return DiagInvalid(DiagCopyAssign);
else if (MD && MD->isMoveAssignmentOperator())
return DiagInvalid(DiagMoveAssign);
+ // [basic.start.main]p3: "The function main shall not be a coroutine."
else if (FD->isMain())
return DiagInvalid(DiagMain);
// Emit a diagnostics for each of the following conditions which is not met.
+ // [expr.const]p2: "An expression e is a core constant expression unless the
+ // evaluation of e [...] would evaluate one of the following expressions:
+ // [...] an await-expression [...] a yield-expression."
if (FD->isConstexpr())
DiagInvalid(DiagConstexpr);
+ // [dcl.spec.auto]p15: "A function declared with a return type that uses a
+ // placeholder type shall not be a coroutine."
if (FD->getReturnType()->isUndeducedType())
DiagInvalid(DiagAutoRet);
+ // [dcl.fct.def.coroutine]p1: "The parameter-declaration-clause of the
+ // coroutine shall not terminate with an ellipsis that is not part of a
+ // parameter-declaration."
if (FD->isVariadic())
DiagInvalid(DiagVarargs);
Modified: cfe/trunk/test/SemaCXX/coroutines.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coroutines.cpp?rev=335420&r1=335419&r2=335420&view=diff
==
--- cfe/trunk/test/SemaCXX/coroutines.cpp (original)
+++ cfe/trunk/test/SemaCXX/coroutines.cpp Sat
[PATCH] D48519: [Sema] isValidCoroutineContext FIXME and citations
This revision was automatically updated to reflect the committed changes.
Closed by commit rC335420: [Sema] isValidCoroutineContext FIXME and citations
(authored by modocache, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48519?vs=152597&id=152598#toc
Repository:
rC Clang
https://reviews.llvm.org/D48519
Files:
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -1,3 +1,6 @@
+// This file contains references to sections of the Coroutines TS, which can be
+// found at http://wg21.link/coroutines.
+
// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -Wunused-result
void no_coroutine_traits_bad_arg_await() {
@@ -320,6 +323,11 @@
typeid(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
}
+// [expr.await]p2: "An await-expression shall not appear in a default argument."
+// FIXME: A better diagnostic would explicitly state that default arguments are
+// not allowed. A user may not understand that this is "outside a function."
+void default_argument(int arg = co_await 0) {} // expected-error {{'co_await' cannot be used outside a function}}
+
constexpr auto constexpr_deduced_return_coroutine() {
co_yield 0; // expected-error {{'co_yield' cannot be used in a constexpr function}}
// expected-error@-1 {{'co_yield' cannot be used in a function with a deduced return type}}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -9,6 +9,9 @@
//
// This file implements semantic analysis for C++ Coroutines.
//
+// This file contains references to sections of the Coroutines TS, which
+// can be found at http://wg21.link/coroutines.
+//
//===--===//
#include "CoroutineStmtBuilder.h"
@@ -196,13 +199,25 @@
static bool isValidCoroutineContext(Sema &S, SourceLocation Loc,
StringRef Keyword) {
- // 'co_await' and 'co_yield' are not permitted in unevaluated operands.
+ // 'co_await' and 'co_yield' are not permitted in unevaluated operands,
+ // such as subexpressions of \c sizeof.
+ //
+ // [expr.await]p2, emphasis added: "An await-expression shall appear only in
+ // a *potentially evaluated* expression within the compound-statement of a
+ // function-body outside of a handler [...] A context within a function where
+ // an await-expression can appear is called a suspension context of the
+ // function." And per [expr.yield]p1: "A yield-expression shall appear only
+ // within a suspension context of a function."
if (S.isUnevaluatedContext()) {
S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
return false;
}
- // Any other usage must be within a function.
+ // Per [expr.await]p2, any other usage must be within a function.
+ // FIXME: This also covers [expr.await]p2: "An await-expression shall not
+ // appear in a default argument." But the diagnostic QoI here could be
+ // improved to inform the user that default arguments specifically are not
+ // allowed.
auto *FD = dyn_cast(S.CurContext);
if (!FD) {
S.Diag(Loc, isa(S.CurContext)
@@ -233,22 +248,37 @@
// Diagnose when a constructor, destructor, copy/move assignment operator,
// or the function 'main' are declared as a coroutine.
auto *MD = dyn_cast(FD);
+ // [class.ctor]p6: "A constructor shall not be a coroutine."
if (MD && isa(MD))
return DiagInvalid(DiagCtor);
+ // [class.dtor]p17: "A destructor shall not be a coroutine."
else if (MD && isa(MD))
return DiagInvalid(DiagDtor);
+ // N4499 [special]p6: "A special member function shall not be a coroutine."
+ // Per C++ [special]p1, special member functions are the "default constructor,
+ // copy constructor and copy assignment operator, move constructor and move
+ // assignment operator, and destructor."
else if (MD && MD->isCopyAssignmentOperator())
return DiagInvalid(DiagCopyAssign);
else if (MD && MD->isMoveAssignmentOperator())
return DiagInvalid(DiagMoveAssign);
+ // [basic.start.main]p3: "The function main shall not be a coroutine."
else if (FD->isMain())
return DiagInvalid(DiagMain);
// Emit a diagnostics for each of the following conditions which is not met.
+ // [expr.const]p2: "An expression e is a core constant expression unless the
+ // evaluation of e [...] would evaluate one of the following expressions:
+ // [...] an await-expression [...] a yield-expression."
if (FD->isConstexpr())
DiagInvalid(DiagConstexpr);
+ // [dcl.spec.auto]p15: "A function declared with a return type that uses a
+ // placeholder type shall not be a coroutine."
if (FD->getReturnType()->isUndeducedType
[PATCH] D47673: [Coroutines] Less IR for noexcept await_resume
modocache updated this revision to Diff 152599.
modocache added a comment.
Great, thanks @GorNishanov! I moved the 'can throw' logic into a function
called 'memberCallExpressionCanThrow', to convey that some dyn_cast'ing is
going on.
Repository:
rC Clang
https://reviews.llvm.org/D47673
Files:
lib/CodeGen/CGCoroutine.cpp
test/CodeGenCoroutines/coro-await-resume-eh.cpp
Index: test/CodeGenCoroutines/coro-await-resume-eh.cpp
===
--- test/CodeGenCoroutines/coro-await-resume-eh.cpp
+++ test/CodeGenCoroutines/coro-await-resume-eh.cpp
@@ -18,18 +18,18 @@
void await_resume() { throw 42; }
};
-struct task {
+struct throwing_task {
struct promise_type {
-task get_return_object() { return task{}; }
+auto get_return_object() { return throwing_task{}; }
auto initial_suspend() { return throwing_awaitable{}; }
auto final_suspend() { return coro::suspend_never{}; }
void return_void() {}
void unhandled_exception() {}
};
};
// CHECK-LABEL: define void @_Z1fv()
-task f() {
+throwing_task f() {
// A variable RESUMETHREW is used to keep track of whether the body
// of 'await_resume' threw an exception. Exceptions thrown in
// 'await_resume' are unwound to RESUMELPAD.
@@ -50,7 +50,7 @@
// CHECK: [[RESUMELPAD]]:
// CHECK: br label %[[RESUMECATCH:.+]]
// CHECK: [[RESUMECATCH]]:
- // CHECK: invoke void @_ZN4task12promise_type19unhandled_exceptionEv
+ // CHECK: invoke void @_ZN13throwing_task12promise_type19unhandled_exceptionEv
// CHECK-NEXT: to label %[[RESUMEENDCATCH:.+]] unwind label
// CHECK: [[RESUMEENDCATCH]]:
// CHECK-NEXT: invoke void @__cxa_end_catch()
@@ -67,15 +67,42 @@
// CHECK-NEXT: br i1 %[[RESUMETHREWLOAD]], label %[[RESUMEDCONT:.+]], label %[[RESUMEDBODY:.+]]
// CHECK: [[RESUMEDBODY]]:
- // CHECK: invoke void @_ZN4task12promise_type11return_voidEv
+ // CHECK: invoke void @_ZN13throwing_task12promise_type11return_voidEv
// CHECK-NEXT: to label %[[REDUMEDBODYCONT:.+]] unwind label
// CHECK: [[REDUMEDBODYCONT]]:
// CHECK-NEXT: br label %[[COROFINAL:.+]]
// CHECK: [[RESUMEDCONT]]:
// CHECK-NEXT: br label %[[COROFINAL]]
// CHECK: [[COROFINAL]]:
- // CHECK-NEXT: invoke void @_ZN4task12promise_type13final_suspendEv
+ // CHECK-NEXT: invoke void @_ZN13throwing_task12promise_type13final_suspendEv
+ co_return;
+}
+
+struct noexcept_awaitable {
+ bool await_ready() { return true; }
+ void await_suspend(coro::coroutine_handle<>) {}
+ void await_resume() noexcept {}
+};
+
+struct noexcept_task {
+ struct promise_type {
+auto get_return_object() { return noexcept_task{}; }
+auto initial_suspend() { return noexcept_awaitable{}; }
+auto final_suspend() { return coro::suspend_never{}; }
+void return_void() {}
+void unhandled_exception() {}
+ };
+};
+
+// CHECK-LABEL: define void @_Z1gv()
+noexcept_task g() {
+ // If the await_resume function is marked as noexcept, none of the additional
+ // conditions that are present in f() above are added to the IR.
+ // This means that no i1 are stored before or after calling await_resume:
+ // CHECK: init.ready:
+ // CHECK-NEXT: call void @_ZN18noexcept_awaitable12await_resumeEv
+ // CHECK-NOT: store i1 false, i1*
co_return;
}
Index: lib/CodeGen/CGCoroutine.cpp
===
--- lib/CodeGen/CGCoroutine.cpp
+++ lib/CodeGen/CGCoroutine.cpp
@@ -130,6 +130,16 @@
return Prefix;
}
+static bool memberCallExpressionCanThrow(const Expr *E) {
+ if (const auto *CE = dyn_cast(E))
+if (const auto *Proto =
+CE->getMethodDecl()->getType()->getAs())
+ if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
+ Proto->canThrow() == CT_Cannot)
+return false;
+ return true;
+}
+
// Emit suspend expression which roughly looks like:
//
// auto && x = CommonExpr();
@@ -217,8 +227,12 @@
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
+
+ // Exception handling requires additional IR. If the 'await_resume' function
+ // is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init) {
+ if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
+ memberCallExpressionCanThrow(S.getResumeExpr())) {
Coro.ResumeEHVar =
CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
Builder.CreateFlagStore(true, Coro.ResumeEHVar);
@@ -625,12 +639,20 @@
CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
if (CurCoro.Data->ExceptionHandler) {
- BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
- BasicBlock *ContBB = createBasicBlock("coro.resumed.cont");
- Value *SkipBody =
- Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar, "coro.resumed.eh");
- Builder.CreateCondBr(SkipBody, ContBB, BodyBB);
- EmitBlock(B
[PATCH] D48521: [analyzer] Highlight STL object destruction in MallocChecker
rnkovacs created this revision.
rnkovacs added reviewers: NoQ, xazax.hun, george.karpenkov, dcoughlin.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, szepet,
baloghadamsoftware, whisperity.
Extend `MallocBugVisitor` to place a note at the point where objects with
`AF_InternalBuffer` allocation family are destroyed.
Repository:
rC Clang
https://reviews.llvm.org/D48521
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/dangling-internal-buffer.cpp
Index: test/Analysis/dangling-internal-buffer.cpp
===
--- test/Analysis/dangling-internal-buffer.cpp
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -26,7 +26,7 @@
{
std::string s;
c = s.c_str();
- }
+ } // expected-note {{Internal buffer is released because the object was destroyed}}
consume(c); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
@@ -36,7 +36,7 @@
{
std::wstring ws;
w = ws.c_str();
- }
+ } // expected-note {{Internal buffer is released because the object was destroyed}}
consume(w); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
@@ -46,7 +46,7 @@
{
std::u16string s16;
c16 = s16.c_str();
- }
+ } // expected-note {{Internal buffer is released because the object was destroyed}}
consume(c16); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
@@ -56,7 +56,7 @@
{
std::u32string s32;
c32 = s32.c_str();
- }
+ } // expected-note {{Internal buffer is released because the object was destroyed}}
consume(c32); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -478,11 +478,10 @@
SPrev->isAllocatedOfSizeZero(;
}
-inline bool isReleased(const RefState *S, const RefState *SPrev,
- const Stmt *Stmt) {
+inline bool isReleased(const RefState *S, const RefState *SPrev) {
// Did not track -> released. Other state (allocated) -> released.
- return (Stmt && (isa(Stmt) || isa(Stmt)) &&
- (S && S->isReleased()) && (!SPrev || !SPrev->isReleased()));
+ // The statement associated with the release might be missing.
+ return (S && S->isReleased()) && (!SPrev || !SPrev->isReleased());
}
inline bool isRelinquished(const RefState *S, const RefState *SPrev,
@@ -2851,8 +2850,17 @@
std::shared_ptr MallocChecker::MallocBugVisitor::VisitNode(
const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC,
BugReport &BR) {
+
+ ProgramStateRef state = N->getState();
+ ProgramStateRef statePrev = PrevN->getState();
+
+ const RefState *RS = state->get(Sym);
+ const RefState *RSPrev = statePrev->get(Sym);
+
const Stmt *S = PathDiagnosticLocation::getStmt(N);
- if (!S)
+ // When dealing with STL containers, we sometimes want to give a note
+ // even if the statement is missing.
+ if (!S && (!RS || RS->getAllocationFamily() != AF_InternalBuffer))
return nullptr;
const LocationContext *CurrentLC = N->getLocationContext();
@@ -2877,14 +2885,6 @@
}
}
- ProgramStateRef state = N->getState();
- ProgramStateRef statePrev = PrevN->getState();
-
- const RefState *RS = state->get(Sym);
- const RefState *RSPrev = statePrev->get(Sym);
- if (!RS)
-return nullptr;
-
// FIXME: We will eventually need to handle non-statement-based events
// (__attribute__((cleanup))).
@@ -2896,8 +2896,23 @@
Msg = "Memory is allocated";
StackHint = new StackHintGeneratorForSymbol(Sym,
"Returned allocated memory");
-} else if (isReleased(RS, RSPrev, S)) {
- Msg = "Memory is released";
+} else if (isReleased(RS, RSPrev)) {
+ const auto Family = RS->getAllocationFamily();
+ switch(Family) {
+case AF_Alloca:
+case AF_Malloc:
+case AF_CXXNew:
+case AF_CXXNewArray:
+case AF_IfNameIndex:
+ Msg = "Memory is released";
+ break;
+case AF_InternalBuffer:
+ Msg = "Internal buffer is released because the object was destroyed";
+ break;
+case AF_None:
+default:
+ llvm_unreachable("unhandled allocation family");
+ }
StackHint = new StackHintGeneratorForSymbol(Sym,
"Returning; memory was released");
@@ -2968,8 +2983,18 @@
assert(StackHint);
// Generate the extra diagnostic.
- PathDiagnosticLocation Pos(S, BRC.getSourceManager(
r335422 - [Coroutines] Less IR for noexcept await_resume
Author: modocache
Date: Sat Jun 23 11:57:26 2018
New Revision: 335422
URL: http://llvm.org/viewvc/llvm-project?rev=335422&view=rev
Log:
[Coroutines] Less IR for noexcept await_resume
Summary:
In his review of https://reviews.llvm.org/D45860, @GorNishanov suggested
avoiding generating additional exception-handling IR in the case that
the resume function was marked as 'noexcept', and exceptions could not
occur. This implements that suggestion.
Test Plan: `check-clang`
Reviewers: GorNishanov, EricWF
Reviewed By: GorNishanov
Subscribers: cfe-commits, GorNishanov
Differential Revision: https://reviews.llvm.org/D47673
Modified:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp
Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=335422&r1=335421&r2=335422&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Sat Jun 23 11:57:26 2018
@@ -130,6 +130,16 @@ static SmallString<32> buildSuspendPrefi
return Prefix;
}
+static bool memberCallExpressionCanThrow(const Expr *E) {
+ if (const auto *CE = dyn_cast(E))
+if (const auto *Proto =
+CE->getMethodDecl()->getType()->getAs())
+ if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
+ Proto->canThrow() == CT_Cannot)
+return false;
+ return true;
+}
+
// Emit suspend expression which roughly looks like:
//
// auto && x = CommonExpr();
@@ -217,8 +227,12 @@ static LValueOrRValue emitSuspendExpress
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
+
+ // Exception handling requires additional IR. If the 'await_resume' function
+ // is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init) {
+ if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
+ memberCallExpressionCanThrow(S.getResumeExpr())) {
Coro.ResumeEHVar =
CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
Builder.CreateFlagStore(true, Coro.ResumeEHVar);
@@ -625,12 +639,20 @@ void CodeGenFunction::EmitCoroutineBody(
CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
if (CurCoro.Data->ExceptionHandler) {
- BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
- BasicBlock *ContBB = createBasicBlock("coro.resumed.cont");
- Value *SkipBody =
- Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar, "coro.resumed.eh");
- Builder.CreateCondBr(SkipBody, ContBB, BodyBB);
- EmitBlock(BodyBB);
+ // If we generated IR to record whether an exception was thrown from
+ // 'await_resume', then use that IR to determine whether the coroutine
+ // body should be skipped.
+ // If we didn't generate the IR (perhaps because 'await_resume' was
marked
+ // as 'noexcept'), then we skip this check.
+ BasicBlock *ContBB = nullptr;
+ if (CurCoro.Data->ResumeEHVar) {
+BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
+ContBB = createBasicBlock("coro.resumed.cont");
+Value *SkipBody = Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar,
+ "coro.resumed.eh");
+Builder.CreateCondBr(SkipBody, ContBB, BodyBB);
+EmitBlock(BodyBB);
+ }
auto Loc = S.getLocStart();
CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
@@ -642,7 +664,8 @@ void CodeGenFunction::EmitCoroutineBody(
emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
ExitCXXTryStmt(*TryStmt);
- EmitBlock(ContBB);
+ if (ContBB)
+EmitBlock(ContBB);
}
else {
emitBodyAndFallthrough(*this, S, S.getBody());
Modified: cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp?rev=335422&r1=335421&r2=335422&view=diff
==
--- cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp (original)
+++ cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp Sat Jun 23
11:57:26 2018
@@ -18,9 +18,9 @@ struct throwing_awaitable {
void await_resume() { throw 42; }
};
-struct task {
+struct throwing_task {
struct promise_type {
-task get_return_object() { return task{}; }
+auto get_return_object() { return throwing_task{}; }
auto initial_suspend() { return throwing_awaitable{}; }
auto final_suspend() { return coro::suspend_never{}; }
void return_void() {}
@@ -29,7 +29,7 @@ struct task {
};
// CHECK-LABEL: define void @_Z1fv()
-task f() {
+throwing_task f() {
// A variable RESUMETHREW is used to keep track of whether the body
// of 'await_resume' threw an exceptio
[PATCH] D47673: [Coroutines] Less IR for noexcept await_resume
This revision was automatically updated to reflect the committed changes.
Closed by commit rC335422: [Coroutines] Less IR for noexcept await_resume
(authored by modocache, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47673?vs=152599&id=152602#toc
Repository:
rC Clang
https://reviews.llvm.org/D47673
Files:
lib/CodeGen/CGCoroutine.cpp
test/CodeGenCoroutines/coro-await-resume-eh.cpp
Index: test/CodeGenCoroutines/coro-await-resume-eh.cpp
===
--- test/CodeGenCoroutines/coro-await-resume-eh.cpp
+++ test/CodeGenCoroutines/coro-await-resume-eh.cpp
@@ -18,18 +18,18 @@
void await_resume() { throw 42; }
};
-struct task {
+struct throwing_task {
struct promise_type {
-task get_return_object() { return task{}; }
+auto get_return_object() { return throwing_task{}; }
auto initial_suspend() { return throwing_awaitable{}; }
auto final_suspend() { return coro::suspend_never{}; }
void return_void() {}
void unhandled_exception() {}
};
};
// CHECK-LABEL: define void @_Z1fv()
-task f() {
+throwing_task f() {
// A variable RESUMETHREW is used to keep track of whether the body
// of 'await_resume' threw an exception. Exceptions thrown in
// 'await_resume' are unwound to RESUMELPAD.
@@ -50,7 +50,7 @@
// CHECK: [[RESUMELPAD]]:
// CHECK: br label %[[RESUMECATCH:.+]]
// CHECK: [[RESUMECATCH]]:
- // CHECK: invoke void @_ZN4task12promise_type19unhandled_exceptionEv
+ // CHECK: invoke void @_ZN13throwing_task12promise_type19unhandled_exceptionEv
// CHECK-NEXT: to label %[[RESUMEENDCATCH:.+]] unwind label
// CHECK: [[RESUMEENDCATCH]]:
// CHECK-NEXT: invoke void @__cxa_end_catch()
@@ -67,15 +67,42 @@
// CHECK-NEXT: br i1 %[[RESUMETHREWLOAD]], label %[[RESUMEDCONT:.+]], label %[[RESUMEDBODY:.+]]
// CHECK: [[RESUMEDBODY]]:
- // CHECK: invoke void @_ZN4task12promise_type11return_voidEv
+ // CHECK: invoke void @_ZN13throwing_task12promise_type11return_voidEv
// CHECK-NEXT: to label %[[REDUMEDBODYCONT:.+]] unwind label
// CHECK: [[REDUMEDBODYCONT]]:
// CHECK-NEXT: br label %[[COROFINAL:.+]]
// CHECK: [[RESUMEDCONT]]:
// CHECK-NEXT: br label %[[COROFINAL]]
// CHECK: [[COROFINAL]]:
- // CHECK-NEXT: invoke void @_ZN4task12promise_type13final_suspendEv
+ // CHECK-NEXT: invoke void @_ZN13throwing_task12promise_type13final_suspendEv
+ co_return;
+}
+
+struct noexcept_awaitable {
+ bool await_ready() { return true; }
+ void await_suspend(coro::coroutine_handle<>) {}
+ void await_resume() noexcept {}
+};
+
+struct noexcept_task {
+ struct promise_type {
+auto get_return_object() { return noexcept_task{}; }
+auto initial_suspend() { return noexcept_awaitable{}; }
+auto final_suspend() { return coro::suspend_never{}; }
+void return_void() {}
+void unhandled_exception() {}
+ };
+};
+
+// CHECK-LABEL: define void @_Z1gv()
+noexcept_task g() {
+ // If the await_resume function is marked as noexcept, none of the additional
+ // conditions that are present in f() above are added to the IR.
+ // This means that no i1 are stored before or after calling await_resume:
+ // CHECK: init.ready:
+ // CHECK-NEXT: call void @_ZN18noexcept_awaitable12await_resumeEv
+ // CHECK-NOT: store i1 false, i1*
co_return;
}
Index: lib/CodeGen/CGCoroutine.cpp
===
--- lib/CodeGen/CGCoroutine.cpp
+++ lib/CodeGen/CGCoroutine.cpp
@@ -130,6 +130,16 @@
return Prefix;
}
+static bool memberCallExpressionCanThrow(const Expr *E) {
+ if (const auto *CE = dyn_cast(E))
+if (const auto *Proto =
+CE->getMethodDecl()->getType()->getAs())
+ if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
+ Proto->canThrow() == CT_Cannot)
+return false;
+ return true;
+}
+
// Emit suspend expression which roughly looks like:
//
// auto && x = CommonExpr();
@@ -217,8 +227,12 @@
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
+
+ // Exception handling requires additional IR. If the 'await_resume' function
+ // is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init) {
+ if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
+ memberCallExpressionCanThrow(S.getResumeExpr())) {
Coro.ResumeEHVar =
CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
Builder.CreateFlagStore(true, Coro.ResumeEHVar);
@@ -625,12 +639,20 @@
CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
if (CurCoro.Data->ExceptionHandler) {
- BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
- BasicBlock *ContBB = createBasicBlock("coro.resumed.cont");
- Value *SkipBody =
- Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar, "coro.resumed.eh");
- Builder.CreateCondBr(SkipBody,
[PATCH] D47673: [Coroutines] Less IR for noexcept await_resume
This revision was automatically updated to reflect the committed changes.
Closed by commit rL335422: [Coroutines] Less IR for noexcept await_resume
(authored by modocache, committed by ).
Repository:
rL LLVM
https://reviews.llvm.org/D47673
Files:
cfe/trunk/lib/CodeGen/CGCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp
Index: cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-await-resume-eh.cpp
@@ -18,18 +18,18 @@
void await_resume() { throw 42; }
};
-struct task {
+struct throwing_task {
struct promise_type {
-task get_return_object() { return task{}; }
+auto get_return_object() { return throwing_task{}; }
auto initial_suspend() { return throwing_awaitable{}; }
auto final_suspend() { return coro::suspend_never{}; }
void return_void() {}
void unhandled_exception() {}
};
};
// CHECK-LABEL: define void @_Z1fv()
-task f() {
+throwing_task f() {
// A variable RESUMETHREW is used to keep track of whether the body
// of 'await_resume' threw an exception. Exceptions thrown in
// 'await_resume' are unwound to RESUMELPAD.
@@ -50,7 +50,7 @@
// CHECK: [[RESUMELPAD]]:
// CHECK: br label %[[RESUMECATCH:.+]]
// CHECK: [[RESUMECATCH]]:
- // CHECK: invoke void @_ZN4task12promise_type19unhandled_exceptionEv
+ // CHECK: invoke void @_ZN13throwing_task12promise_type19unhandled_exceptionEv
// CHECK-NEXT: to label %[[RESUMEENDCATCH:.+]] unwind label
// CHECK: [[RESUMEENDCATCH]]:
// CHECK-NEXT: invoke void @__cxa_end_catch()
@@ -67,15 +67,42 @@
// CHECK-NEXT: br i1 %[[RESUMETHREWLOAD]], label %[[RESUMEDCONT:.+]], label %[[RESUMEDBODY:.+]]
// CHECK: [[RESUMEDBODY]]:
- // CHECK: invoke void @_ZN4task12promise_type11return_voidEv
+ // CHECK: invoke void @_ZN13throwing_task12promise_type11return_voidEv
// CHECK-NEXT: to label %[[REDUMEDBODYCONT:.+]] unwind label
// CHECK: [[REDUMEDBODYCONT]]:
// CHECK-NEXT: br label %[[COROFINAL:.+]]
// CHECK: [[RESUMEDCONT]]:
// CHECK-NEXT: br label %[[COROFINAL]]
// CHECK: [[COROFINAL]]:
- // CHECK-NEXT: invoke void @_ZN4task12promise_type13final_suspendEv
+ // CHECK-NEXT: invoke void @_ZN13throwing_task12promise_type13final_suspendEv
+ co_return;
+}
+
+struct noexcept_awaitable {
+ bool await_ready() { return true; }
+ void await_suspend(coro::coroutine_handle<>) {}
+ void await_resume() noexcept {}
+};
+
+struct noexcept_task {
+ struct promise_type {
+auto get_return_object() { return noexcept_task{}; }
+auto initial_suspend() { return noexcept_awaitable{}; }
+auto final_suspend() { return coro::suspend_never{}; }
+void return_void() {}
+void unhandled_exception() {}
+ };
+};
+
+// CHECK-LABEL: define void @_Z1gv()
+noexcept_task g() {
+ // If the await_resume function is marked as noexcept, none of the additional
+ // conditions that are present in f() above are added to the IR.
+ // This means that no i1 are stored before or after calling await_resume:
+ // CHECK: init.ready:
+ // CHECK-NEXT: call void @_ZN18noexcept_awaitable12await_resumeEv
+ // CHECK-NOT: store i1 false, i1*
co_return;
}
Index: cfe/trunk/lib/CodeGen/CGCoroutine.cpp
===
--- cfe/trunk/lib/CodeGen/CGCoroutine.cpp
+++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp
@@ -130,6 +130,16 @@
return Prefix;
}
+static bool memberCallExpressionCanThrow(const Expr *E) {
+ if (const auto *CE = dyn_cast(E))
+if (const auto *Proto =
+CE->getMethodDecl()->getType()->getAs())
+ if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
+ Proto->canThrow() == CT_Cannot)
+return false;
+ return true;
+}
+
// Emit suspend expression which roughly looks like:
//
// auto && x = CommonExpr();
@@ -217,8 +227,12 @@
// Emit await_resume expression.
CGF.EmitBlock(ReadyBlock);
+
+ // Exception handling requires additional IR. If the 'await_resume' function
+ // is marked as 'noexcept', we avoid generating this additional IR.
CXXTryStmt *TryStmt = nullptr;
- if (Coro.ExceptionHandler && Kind == AwaitKind::Init) {
+ if (Coro.ExceptionHandler && Kind == AwaitKind::Init &&
+ memberCallExpressionCanThrow(S.getResumeExpr())) {
Coro.ResumeEHVar =
CGF.CreateTempAlloca(Builder.getInt1Ty(), Prefix + Twine("resume.eh"));
Builder.CreateFlagStore(true, Coro.ResumeEHVar);
@@ -625,12 +639,20 @@
CurCoro.Data->CurrentAwaitKind = AwaitKind::Normal;
if (CurCoro.Data->ExceptionHandler) {
- BasicBlock *BodyBB = createBasicBlock("coro.resumed.body");
- BasicBlock *ContBB = createBasicBlock("coro.resumed.cont");
- Value *SkipBody =
- Builder.CreateFlagLoad(CurCoro.Data->ResumeEHVar, "coro.resumed.eh");
- Builder.CreateCondBr(SkipBody, ContB
[PATCH] D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker
rnkovacs created this revision.
rnkovacs added reviewers: NoQ, xazax.hun, george.karpenkov, dcoughlin.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, szepet,
baloghadamsoftware, whisperity.
Add a bug visitor to `DanglingInternalBuffer` checker that places a note at the
point where the dangling pointer was obtained.
The visitor is handed over to `MallocChecker` and attached to the report there.
Repository:
rC Clang
https://reviews.llvm.org/D48522
Files:
lib/StaticAnalyzer/Checkers/AllocationState.h
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/dangling-internal-buffer.cpp
Index: test/Analysis/dangling-internal-buffer.cpp
===
--- test/Analysis/dangling-internal-buffer.cpp
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -25,7 +25,7 @@
const char *c;
{
std::string s;
-c = s.c_str();
+c = s.c_str(); // expected-note {{Dangling buffer was obtained here}}
} // expected-note {{Internal buffer is released because the object was destroyed}}
consume(c); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
@@ -35,7 +35,7 @@
const wchar_t *w;
{
std::wstring ws;
-w = ws.c_str();
+w = ws.c_str(); // expected-note {{Dangling buffer was obtained here}}
} // expected-note {{Internal buffer is released because the object was destroyed}}
consume(w); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
@@ -45,7 +45,7 @@
const char16_t *c16;
{
std::u16string s16;
-c16 = s16.c_str();
+c16 = s16.c_str(); // expected-note {{Dangling buffer was obtained here}}
} // expected-note {{Internal buffer is released because the object was destroyed}}
consume(c16); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
@@ -55,7 +55,7 @@
const char32_t *c32;
{
std::u32string s32;
-c32 = s32.c_str();
+c32 = s32.c_str(); // expected-note {{Dangling buffer was obtained here}}
} // expected-note {{Internal buffer is released because the object was destroyed}}
consume(c32); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1988,6 +1988,11 @@
R->markInteresting(Sym);
R->addRange(Range);
R->addVisitor(llvm::make_unique(Sym));
+
+const RefState *RS = C.getState()->get(Sym);
+if (RS->getAllocationFamily() == AF_InternalBuffer)
+ R->addVisitor(allocation_state::getDanglingBufferBRVisitor());
+
C.emitReport(std::move(R));
}
}
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -31,6 +31,28 @@
CallDescription CStrFn;
public:
+ class DanglingBufferBRVisitor
+ : public BugReporterVisitorImpl {
+bool Satisfied;
+
+ public:
+DanglingBufferBRVisitor() : Satisfied(false) {}
+
+static void *getTag() {
+ static int Tag = 0;
+ return &Tag;
+}
+
+void Profile(llvm::FoldingSetNodeID &ID) const override {
+ ID.AddPointer(getTag());
+}
+
+std::shared_ptr VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+ };
+
DanglingInternalBufferChecker() : CStrFn("c_str") {}
/// Record the connection between the symbol returned by c_str() and the
@@ -103,6 +125,54 @@
C.addTransition(State);
}
+std::shared_ptr
+DanglingInternalBufferChecker::DanglingBufferBRVisitor::VisitNode(
+const ExplodedNode *N, const ExplodedNode *PrevN,
+BugReporterContext &BRC, BugReport &BR) {
+ if (Satisfied)
+return nullptr;
+
+ const Stmt *S = PathDiagnosticLocation::getStmt(N);
+ const auto *MemberCallExpr = dyn_cast_or_null(S);
+ if (!MemberCallExpr)
+return nullptr;
+
+ const Decl *D = MemberCallExpr->getCalleeDecl();
+ const auto *FD = dyn_cast_or_null(D);
+ if (!FD)
+return nullptr;
+
+ const IdentifierInfo *FunI = FD->getIdentifier();
+ if (!FunI)
+return nullptr;
+
+ if (!(FunI->getName() == "c_str"))
+return nullptr;
+
+ Satisfied = true;
+
+ SmallString<256> Buf;
+ llvm::raw_svector_ostream OS(Buf);
+ OS << "Dangling buffer was obtained here";
+ PathDiagnosticLocation Pos(S, BRC.get
[PATCH] D48521: [analyzer] Highlight STL object destruction in MallocChecker
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:483
// Did not track -> released. Other state (allocated) -> released.
- return (Stmt && (isa(Stmt) || isa(Stmt)) &&
- (S && S->isReleased()) && (!SPrev || !SPrev->isReleased()));
+ // The statement associated with the release might be missing.
+ return (S && S->isReleased()) && (!SPrev || !SPrev->isReleased());
To be honest, I am not sure what was the purpose of the AST based checks in the
original version. IMHO, looking at the states only should be sufficient without
examining the AST.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2914
+default:
+ llvm_unreachable("unhandled allocation family");
+ }
I think these messages should be full sentences starting with a capital letter
and ending with a period.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2988
+ if (!S) {
+auto PostImplCall = N->getLocation().getAs();
+if (!PostImplCall.hasValue())
This new code should only be used for `AF_InternalBuffer` allocation family? If
that is the case, maybe adding an assert here and running it on some large
codebases to se if this is triggered would be great.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2989
+auto PostImplCall = N->getLocation().getAs();
+if (!PostImplCall.hasValue())
+ return nullptr;
LLVM's optional can implicitly convert to bool. I think it is perfectly fine to
rely on the implicit conversion here.
Repository:
rC Clang
https://reviews.llvm.org/D48521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker
xazax.hun added a comment.
Regarding the visitor:
Maybe rather than looking at the AST, we should check the states, when we
started to track the returned symbol?
Using your current design you need to check for the AST twice. Once in the
visitor and once in the check.
Also, I wonder if this always give you the right note. Consider the following
example:
void deref_after_scope_char() {
const char *c;
{
std::string s;
c = s.c_str();
}
std::string s;
const char *c2 = s.c_str();
consume(c);
}
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:149
+
+ if (!(FunI->getName() == "c_str"))
+return nullptr;
Why not `!=`?
Repository:
rC Clang
https://reviews.llvm.org/D48522
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker
rnkovacs updated this revision to Diff 152604.
rnkovacs marked an inline comment as done.
rnkovacs added a comment.
Um, sorry, I totally forgot about that. Added your case to the tests.
https://reviews.llvm.org/D48522
Files:
lib/StaticAnalyzer/Checkers/AllocationState.h
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/dangling-internal-buffer.cpp
Index: test/Analysis/dangling-internal-buffer.cpp
===
--- test/Analysis/dangling-internal-buffer.cpp
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -25,17 +25,29 @@
const char *c;
{
std::string s;
-c = s.c_str();
+c = s.c_str(); // expected-note {{Dangling buffer was obtained here}}
+ } // expected-note {{Internal buffer is released because the object was destroyed}}
+ consume(c); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char2() {
+ const char *c;
+ {
+std::string s;
+c = s.c_str(); // expected-note {{Dangling buffer was obtained here}}
} // expected-note {{Internal buffer is released because the object was destroyed}}
+ std::string s;
+ const char *c2 = s.c_str();
consume(c); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
}
void deref_after_scope_wchar_t() {
const wchar_t *w;
{
std::wstring ws;
-w = ws.c_str();
+w = ws.c_str(); // expected-note {{Dangling buffer was obtained here}}
} // expected-note {{Internal buffer is released because the object was destroyed}}
consume(w); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
@@ -45,7 +57,7 @@
const char16_t *c16;
{
std::u16string s16;
-c16 = s16.c_str();
+c16 = s16.c_str(); // expected-note {{Dangling buffer was obtained here}}
} // expected-note {{Internal buffer is released because the object was destroyed}}
consume(c16); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
@@ -55,7 +67,7 @@
const char32_t *c32;
{
std::u32string s32;
-c32 = s32.c_str();
+c32 = s32.c_str(); // expected-note {{Dangling buffer was obtained here}}
} // expected-note {{Internal buffer is released because the object was destroyed}}
consume(c32); // expected-warning {{Use of memory after it is freed}}
// expected-note@-1 {{Use of memory after it is freed}}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1988,6 +1988,11 @@
R->markInteresting(Sym);
R->addRange(Range);
R->addVisitor(llvm::make_unique(Sym));
+
+const RefState *RS = C.getState()->get(Sym);
+if (RS->getAllocationFamily() == AF_InternalBuffer)
+ R->addVisitor(allocation_state::getDanglingBufferBRVisitor(Sym));
+
C.emitReport(std::move(R));
}
}
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -24,13 +24,51 @@
using namespace clang;
using namespace ento;
+// FIXME: c_str() may be called on a string object many times, so it should
+// have a list of symbols associated with it.
+REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, SymbolRef)
+
namespace {
class DanglingInternalBufferChecker : public Checker {
CallDescription CStrFn;
public:
+ class DanglingBufferBRVisitor
+ : public BugReporterVisitorImpl {
+// Tracked pointer to a buffer.
+SymbolRef Sym;
+
+ public:
+DanglingBufferBRVisitor(SymbolRef Sym) : Sym(Sym) {}
+
+static void *getTag() {
+ static int Tag = 0;
+ return &Tag;
+}
+
+void Profile(llvm::FoldingSetNodeID &ID) const override {
+ ID.AddPointer(getTag());
+}
+
+std::shared_ptr VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+
+bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) {
+ bool Found = false;
+ RawPtrMapTy Map = State->get();
+ for (const auto Entry : Map) {
+if (Entry.second == Sym)
+ Found = true;
+ }
+ return Found;
+}
+
+ };
+
DanglingInternalBufferChecker() : CStrFn("c_str") {}
/// Record the connection between the symbol returned by c_str() and the
@@ -44,10 +82,6 @@
} // end anonymous namespace
-//
[PATCH] D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker
xazax.hun added a comment. Looks better, thanks! Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65 +if (Entry.second == Sym) + Found = true; + } Maybe early return here? https://reviews.llvm.org/D48522 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48523: [clang-tidy] Update run-clang-tidy.py with vfsoverlay arg
jdemeule created this revision.
jdemeule added a reviewer: alexfh.
Herald added subscribers: cfe-commits, xazax.hun.
Updating the run-clang-tidy.py script to allow specification of the
vfsoverlay argument to the clang-tidy invocation.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48523
Files:
clang-tidy/tool/run-clang-tidy.py
Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -75,8 +75,8 @@
def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
-header_filter, extra_arg, extra_arg_before, quiet,
-config):
+vfsoverlay, header_filter, extra_arg, extra_arg_before,
+quiet, config):
"""Gets a command line for clang-tidy."""
start = [clang_tidy_binary]
if header_filter is not None:
@@ -98,6 +98,8 @@
for arg in extra_arg_before:
start.append('-extra-arg-before=%s' % arg)
start.append('-p=' + build_path)
+ if vfsoverlay is not None:
+start.append('-vfsoverlay=' + vfsoverlay)
if quiet:
start.append('-quiet')
if config:
@@ -158,9 +160,10 @@
while True:
name = queue.get()
invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
- tmpdir, build_path, args.header_filter,
- args.extra_arg, args.extra_arg_before,
- args.quiet, args.config)
+ tmpdir, build_path, args.vfsoverlay,
+ args.header_filter, args.extra_arg,
+ args.extra_arg_before, args.quiet,
+ args.config)
sys.stdout.write(' '.join(invocation) + '\n')
return_code = subprocess.call(invocation)
if return_code != 0:
@@ -209,6 +212,9 @@
'code after applying fixes')
parser.add_argument('-p', dest='build_path',
help='Path used to read a compile command database.')
+ parser.add_argument('-vfsoverlay', dest='vfsoverlay',
+ help='Overlay the virtual filesystem described by file '
+ 'over the real file system.')
parser.add_argument('-extra-arg', dest='extra_arg',
action='append', default=[],
help='Additional argument to append to the compiler '
Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -75,8 +75,8 @@
def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
-header_filter, extra_arg, extra_arg_before, quiet,
-config):
+vfsoverlay, header_filter, extra_arg, extra_arg_before,
+quiet, config):
"""Gets a command line for clang-tidy."""
start = [clang_tidy_binary]
if header_filter is not None:
@@ -98,6 +98,8 @@
for arg in extra_arg_before:
start.append('-extra-arg-before=%s' % arg)
start.append('-p=' + build_path)
+ if vfsoverlay is not None:
+start.append('-vfsoverlay=' + vfsoverlay)
if quiet:
start.append('-quiet')
if config:
@@ -158,9 +160,10 @@
while True:
name = queue.get()
invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
- tmpdir, build_path, args.header_filter,
- args.extra_arg, args.extra_arg_before,
- args.quiet, args.config)
+ tmpdir, build_path, args.vfsoverlay,
+ args.header_filter, args.extra_arg,
+ args.extra_arg_before, args.quiet,
+ args.config)
sys.stdout.write(' '.join(invocation) + '\n')
return_code = subprocess.call(invocation)
if return_code != 0:
@@ -209,6 +212,9 @@
'code after applying fixes')
parser.add_argument('-p', dest='build_path',
help='Path used to read a compile command database.')
+ parser.add_argument('-vfsoverlay', dest='vfsoverlay',
+ help='Overlay the virtual filesystem described by file '
+ 'over the real file system.')
parser.add_argument('-extra-arg', dest='extra_arg',
action='append', default=[],
help='Additional argument to append to the compiler '
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48524: [ODRHash] Do not put elaborated types in the TypeMap
v.g.vassilev created this revision.
v.g.vassilev added reviewers: rsmith, rtrieu, bruno.
Herald added a subscriber: cfe-commits.
r332281 (https://reviews.llvm.org/D45463) teaches the ASTContext to generate
different elaborated types if there is an owning tag. This exposed a bug with
our odr hasher: we add both types in the `TypeMap` which makes the hashes
different.
This patch is a quick fix for the problem. However, it looks like the `TypeMap`
in a way disables the nice cross-tu hashing logic.
Repository:
rC Clang
https://reviews.llvm.org/D48524
Files:
lib/AST/ODRHash.cpp
test/Modules/Inputs/odr_hash-elaborated-types/first.h
test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap
test/Modules/Inputs/odr_hash-elaborated-types/second.h
test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
test/Modules/odr_hash-elaborated-types.cpp
Index: test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
@@ -0,0 +1,6 @@
+#ifndef _TIME_H
+#define _TIME_H
+
+struct timespec { };
+
+#endif
Index: test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
@@ -0,0 +1,11 @@
+#ifndef _SYS_STAT_H
+#define _SYS_STAT_H
+
+#include "textual_time.h"
+
+struct stat {
+ struct timespec st_atim;
+ struct timespec st_mtim;
+};
+
+#endif
Index: test/Modules/Inputs/odr_hash-elaborated-types/second.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/second.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/second.h
@@ -0,0 +1,6 @@
+#ifndef SECOND
+#define SECOND
+
+#include "textual_stat.h"
+
+#endif
Index: test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap
===
--- test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap
+++ test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap
@@ -0,0 +1,5 @@
+module M {
+ module first { header "first.h" export *}
+ module second { header "second.h" export *}
+ export *
+}
Index: test/Modules/Inputs/odr_hash-elaborated-types/first.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/first.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/first.h
@@ -0,0 +1,6 @@
+#ifndef FIRST
+#define FIRST
+
+#include "textual_time.h"
+
+#endif
Index: test/Modules/odr_hash-elaborated-types.cpp
===
--- test/Modules/odr_hash-elaborated-types.cpp
+++ test/Modules/odr_hash-elaborated-types.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++1z -I%S/Inputs/odr_hash-elaborated-types -verify %s
+// RUN: %clang_cc1 -std=c++1z -fmodules -fmodules-local-submodule-visibility
-fmodule-map-file=%S/Inputs/odr_hash-elaborated-types/module.modulemap
-fmodules-cache-path=%t -x c++ -I%S/Inputs/odr_hash-elaborated-types -verify %s
+
+#include "textual_stat.h"
+
+#include "first.h"
+#include "second.h"
+
+void use() { struct stat value; }
+
+// expected-no-diagnostics
Index: lib/AST/ODRHash.cpp
===
--- lib/AST/ODRHash.cpp
+++ lib/AST/ODRHash.cpp
@@ -749,7 +749,10 @@
void ODRHash::AddType(const Type *T) {
assert(T && "Expecting non-null pointer.");
- auto Result = TypeMap.insert(std::make_pair(T, TypeMap.size()));
+ const Type *TypeInTypeMap = T;
+ if (const ElaboratedType *ElTy = dyn_cast(T))
+TypeInTypeMap = ElTy->getNamedType().getTypePtr();
+ auto Result = TypeMap.insert(std::make_pair(TypeInTypeMap, TypeMap.size()));
ID.AddInteger(Result.first->second);
// On first encounter of a Type pointer, process it. Every time afterwards,
// only the index value is needed.
Index: test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
@@ -0,0 +1,6 @@
+#ifndef _TIME_H
+#define _TIME_H
+
+struct timespec { };
+
+#endif
Index: test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
@@ -0,0 +1,11 @@
+#ifndef _SYS_STAT_H
+#define _SYS_STAT_H
+
+#include "textual_time.h"
+
+struct stat {
+ struct timespec st_atim;
+ struct timespec st_mtim;
+};
+
+#endif
In
[PATCH] D48460: [analyzer] Fix invalidation on C++ const methods.
NoQ added a comment.
Well, i guess that's just one of those mildly infuriating aspects of the AST
and/or the standard :)
> If references are resolved, why are pointers not?
It's kinda understandable. References are simple aliases to glvalues. When you
use a reference, you simply get *the* original glvalue, which is exactly what
the AST gives you: a glvalue expression of object type. Here's what
`Expr::setType()` says:
130 void setType(QualType t) {
131 // In C++, the type of an expression is always adjusted so that it
132 // will not have reference type (C++ [expr]p6). Use
133 // QualType::getNonReferenceType() to retrieve the non-reference
134 // type. Additionally, inspect Expr::isLvalue to determine whether
135 // an expression that is adjusted in this manner should be
136 // considered an lvalue.
137 assert((t.isNull() || !t->isReferenceType()) &&
138"Expressions can't have reference type");
139
140 TR = t;
141 }
This code is still imperfect because it doesn't take dynamic type of the region
into account, which may be more specific than the expression's type. I guess
i'll add a FIXME.
Repository:
rC Clang
https://reviews.llvm.org/D48460
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/AllocationState.h:23-25 +std::unique_ptr +getDanglingBufferBRVisitor(SymbolRef Sym); + I think we should start commenting this stuff up. Like, "This function provides an additional visitor that augments the bug report with information relevant to memory errors caused by misuse of `AF_InternalBuffer` symbols". https://reviews.llvm.org/D48522 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48522: [analyzer] Highlight c_str() call in DanglingInternalBuffer checker
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks good tho!
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:63-64
+ RawPtrMapTy Map = State->get();
+ for (const auto Entry : Map) {
+if (Entry.second == Sym)
+ Found = true;
Interesting, so we don't have access to the region with which the symbol is
associated, so we have to scan the whole map.
Probably we can scan the map only once (eg., in the visitor's consturctor if we
also supply the program state) and then do a direct lookup by region?
Because it's a premature optimization, i'm in favor of a FIXME.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:155
+ llvm::raw_svector_ostream OS(Buf);
+ OS << "Dangling buffer was obtained here";
+ PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
Maybe "pointer to dangling buffer".
https://reviews.llvm.org/D48522
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48521: [analyzer] Highlight STL object destruction in MallocChecker
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Only minor nits, as usual :)
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:483
// Did not track -> released. Other state (allocated) -> released.
- return (Stmt && (isa(Stmt) || isa(Stmt)) &&
- (S && S->isReleased()) && (!SPrev || !SPrev->isReleased()));
+ // The statement associated with the release might be missing.
+ return (S && S->isReleased()) && (!SPrev || !SPrev->isReleased());
xazax.hun wrote:
> To be honest, I am not sure what was the purpose of the AST based checks in
> the original version. IMHO, looking at the states only should be sufficient
> without examining the AST.
Yeah, same. I guess we could replace the check with an assertion (i.e. if
there's statement, it's a call or delete-expression and it's
`AF_InternalBuffer`), and see if anything crashes.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2861
const Stmt *S = PathDiagnosticLocation::getStmt(N);
- if (!S)
+ // When dealing with STL containers, we sometimes want to give a note
+ // even if the statement is missing.
I don't think our check will be limited to STL containers only. We might cover
other containers as well, such as LLVM or WebKit custom strings and string
views.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2914
+default:
+ llvm_unreachable("unhandled allocation family");
+ }
xazax.hun wrote:
> I think these messages should be full sentences starting with a capital
> letter and ending with a period.
Or exclamantion mark! :)
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2991
+ return nullptr;
+Pos = PathDiagnosticLocation(PostImplCall.getValue().getLocation(),
+ BRC.getSourceManager());
`PostImplCall->getLocation()`.
Repository:
rC Clang
https://reviews.llvm.org/D48521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
