<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, Jul 6, 2014 at 4:25 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
On 07/07/2014 01:53, Nico Weber wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: nico<br>
Date: Sun Jul 6 17:53:19 2014<br>
New Revision: 212422<br>
<br>
URL:<a href="http://llvm.org/viewvc/llvm-project?rev=212422&view=rev" target="_blank">http://llvm.org/viewvc/<u></u>llvm-project?rev=212422&view=<u></u>rev</a><br>
Log:<br>
Sema: Check that __leave is contained in a __try block.<br>
<br>
Give scope a SEHTryScope bit, set that in ParseSEHTry(), and let Sema<br>
walk the scope chain to find the SEHTry parent on __leave statements.<br>
(They are rare enough that it seems better to do the walk instead of<br>
giving Scope a SEHTryParent pointer -- this is similar to AtCatchScope.)<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/<u></u>DiagnosticSemaKinds.td<br>
cfe/trunk/include/clang/Sema/<u></u>Scope.h<br>
cfe/trunk/lib/Parse/ParseStmt.<u></u>cpp<br>
cfe/trunk/lib/Sema/Scope.cpp<br>
cfe/trunk/lib/Sema/SemaStmt.<u></u>cpp<br>
cfe/trunk/test/Sema/__try.c<br>
cfe/trunk/test/SemaCXX/__try.<u></u>cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/<u></u>DiagnosticSemaKinds.td<br>
URL:<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=212422&r1=212421&r2=212422&view=diff" target="_blank">http://llvm.org/viewvc/<u></u>llvm-project/cfe/trunk/<u></u>include/clang/Basic/<u></u>DiagnosticSemaKinds.td?rev=<u></u>212422&r1=212421&r2=212422&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/include/clang/Basic/<u></u>DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/<u></u>DiagnosticSemaKinds.td Sun Jul 6 17:53:19 2014<br>
@@ -5155,6 +5155,8 @@ def err_need_header_before_ms_<u></u>uuidof : E<br>
"you need to include <guiddef.h> before using the '__uuidof' operator">;<br>
def err_ms___leave_unimplemented : Error<<br>
"__leave support not implemented yet">;<br>
+def err_ms___leave_not_in___try : Error<<br>
+ "'__leave' statement not in __try block">;<br>
def err_uuidof_without_guid : Error<<br>
"cannot call operator __uuidof on a type with no GUID">;<br>
def err_uuidof_with_multiple_guids : Error<<br>
</blockquote>
<br></div></div>
The triple underscores don't add much here, how about just 'err_ms_leave_not_in_try' and 'err_ms_leave_unimplemented' to match the style of others like 'like the err_uuidof_without_guid' immediately below?</blockquote>
<div><br></div><div>There's a ___ for a __block diagnostic elsewhere in this file. Since there isn't a consistency advantage either way I left it as is for now. (tok::kw___leave has the triple underscore too, so if someone wnats wants to make this all consistent, adding __s to uuidof is probably the way to go.)</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Modified: cfe/trunk/include/clang/Sema/<u></u>Scope.h<br>
URL:<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=212422&r1=212421&r2=212422&view=diff" target="_blank">http://llvm.org/viewvc/<u></u>llvm-project/cfe/trunk/<u></u>include/clang/Sema/Scope.h?<u></u>rev=212422&r1=212421&r2=<u></u>212422&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/include/clang/Sema/<u></u>Scope.h (original)<br>
+++ cfe/trunk/include/clang/Sema/<u></u>Scope.h Sun Jul 6 17:53:19 2014<br>
@@ -114,6 +114,9 @@ public:<br>
/// This scope corresponds to an enum.<br>
EnumScope = 0x40000,<br>
+<br>
+ /// This scope corresponds to a SEH try.<br>
+ SEHTryScope = 0x80000,<br>
};<br>
private:<br>
/// The parent scope for this scope. This is null for the translation-unit<br>
@@ -398,6 +401,9 @@ public:<br>
/// \brief Determine whether this scope is a C++ 'try' block.<br>
bool isTryScope() const { return getFlags() & Scope::TryScope; }<br>
+ /// \brief Determine whether this scope is a SEH '__try' block.<br>
+ bool isSEHTryScope() const { return getFlags() & Scope::SEHTryScope; }<br>
+<br>
/// containedInPrototypeScope - Return true if this or a parent scope<br>
/// is a FunctionPrototypeScope.<br>
bool containedInPrototypeScope() const;<br>
<br>
Modified: cfe/trunk/lib/Parse/ParseStmt.<u></u>cpp<br>
URL:<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=212422&r1=212421&r2=212422&view=diff" target="_blank">http://llvm.org/viewvc/<u></u>llvm-project/cfe/trunk/lib/<u></u>Parse/ParseStmt.cpp?rev=<u></u>212422&r1=212421&r2=212422&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/Parse/ParseStmt.<u></u>cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseStmt.<u></u>cpp Sun Jul 6 17:53:19 2014<br>
@@ -427,7 +427,8 @@ StmtResult Parser::ParseSEHTryBlockCommo<br>
if(Tok.isNot(tok::l_brace))<br>
return StmtError(Diag(Tok, diag::err_expected) << tok::l_brace);<br>
- StmtResult TryBlock(<u></u>ParseCompoundStatement());<br>
+ StmtResult TryBlock(<u></u>ParseCompoundStatement(/*<u></u>isStmtExpr=*/false,<br>
+ Scope::DeclScope | Scope::SEHTryScope));<br>
if(TryBlock.isInvalid())<br>
return TryBlock;<br>
<br>
Modified: cfe/trunk/lib/Sema/Scope.cpp<br>
URL:<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Scope.cpp?rev=212422&r1=212421&r2=212422&view=diff" target="_blank">http://llvm.org/viewvc/<u></u>llvm-project/cfe/trunk/lib/<u></u>Sema/Scope.cpp?rev=212422&r1=<u></u>212421&r2=212422&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/Sema/Scope.cpp (original)<br>
+++ cfe/trunk/lib/Sema/Scope.cpp Sun Jul 6 17:53:19 2014<br>
@@ -182,6 +182,9 @@ void Scope::dumpImpl(raw_ostream &OS) co<br>
} else if (Flags & FnTryCatchScope) {<br>
OS << "FnTryCatchScope";<br>
Flags &= ~FnTryCatchScope;<br>
+ } else if (Flags & SEHTryScope) {<br>
+ OS << "SEHTryScope";<br>
+ Flags &= ~SEHTryScope;<br>
} else if (Flags & OpenMPDirectiveScope) {<br>
OS << "OpenMPDirectiveScope";<br>
Flags &= ~OpenMPDirectiveScope;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaStmt.<u></u>cpp<br>
URL:<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=212422&r1=212421&r2=212422&view=diff" target="_blank">http://llvm.org/viewvc/<u></u>llvm-project/cfe/trunk/lib/<u></u>Sema/SemaStmt.cpp?rev=212422&<u></u>r1=212421&r2=212422&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/Sema/SemaStmt.<u></u>cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaStmt.<u></u>cpp Sun Jul 6 17:53:19 2014<br>
@@ -3279,6 +3279,12 @@ Sema::ActOnSEHFinallyBlock(<u></u>SourceLocatio<br>
StmtResult<br>
Sema::ActOnSEHLeaveStmt(<u></u>SourceLocation Loc, Scope *CurScope) {<br>
+ Scope *SEHTryParent = CurScope;<br>
+ while (SEHTryParent && !SEHTryParent->isSEHTryScope()<u></u>)<br>
+ SEHTryParent = SEHTryParent->getParent();<br>
+ if (!SEHTryParent)<br>
+ return StmtError(Diag(Loc, diag::err_ms___leave_not_in___<u></u>try));<br>
+<br>
return StmtError(Diag(Loc, diag::err_ms___leave_<u></u>unimplemented));<br>
}<br>
<br>
Modified: cfe/trunk/test/Sema/__try.c<br>
URL:<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/__try.c?rev=212422&r1=212421&r2=212422&view=diff" target="_blank">http://llvm.org/viewvc/<u></u>llvm-project/cfe/trunk/test/<u></u>Sema/__try.c?rev=212422&r1=<u></u>212421&r2=212422&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/test/Sema/__try.c (original)<br>
+++ cfe/trunk/test/Sema/__try.c Sun Jul 6 17:53:19 2014<br>
@@ -172,26 +172,23 @@ void TEST() {<br>
}<br>
void test___leave() {<br>
</blockquote>
<br></div></div>
Since this is a sema test, how about naming it after the semantics being tested -- void test_seh_leave_stmt() ?<br></blockquote><div><br></div><div>Sure, done as part of r212425.</div><div><br>Thanks!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Alp.<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
- // FIXME: should say "__leave stmt not in __try block":<br>
- __leave; // expected-error{{not implemented yet}}<br>
+ __leave; // expected-error{{'__leave' statement not in __try block}}<br>
+<br>
__try {<br>
// FIXME: should be fine<br>
__leave; // expected-error{{not implemented yet}}<br>
// FIXME: should say "expected ';' after __leave statement"<br>
__leave 4; // expected-error{{not implemented yet}} expected-warning{{expression result unused}}<br>
} __except(1) {<br>
- // FIXME: should say "__leave stmt not in __try block":<br>
- __leave; // expected-error{{not implemented yet}}<br>
+ __leave; // expected-error{{'__leave' statement not in __try block}}<br>
}<br>
__try {<br>
// FIXME: should be fine<br>
__leave; // expected-error{{not implemented yet}}<br>
} __finally {<br>
- // FIXME: should say "__leave stmt not in __try block":<br>
- __leave; // expected-error{{not implemented yet}}<br>
+ __leave; // expected-error{{'__leave' statement not in __try block}}<br>
}<br>
- // FIXME: should say "__leave stmt not in __try block":<br>
- __leave; // expected-error{{not implemented yet}}<br>
+ __leave; // expected-error{{'__leave' statement not in __try block}}<br>
}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/__try.<u></u>cpp<br>
URL:<a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/__try.cpp?rev=212422&r1=212421&r2=212422&view=diff" target="_blank">http://llvm.org/viewvc/<u></u>llvm-project/cfe/trunk/test/<u></u>SemaCXX/__try.cpp?rev=212422&<u></u>r1=212421&r2=212422&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/test/SemaCXX/__try.<u></u>cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/__try.<u></u>cpp Sun Jul 6 17:53:19 2014<br>
@@ -83,8 +83,7 @@ void test___leave() {<br>
// Clang accepts try with __finally. MSVC doesn't. (Maybe a Borland thing?)<br>
// __leave in mixed blocks isn't supported.<br>
try {<br>
- // FIXME: should say "__leave stmt not in __try block":<br>
- __leave; // expected-error{{not implemented yet}}<br>
+ __leave; // expected-error{{'__leave' statement not in __try block}}<br>
} __finally {<br>
}<br>
}<br>
<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</blockquote>
<br></div></div><span class="HOEnZb"><font color="#888888">
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>