<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 14, 2014 at 9:53 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Could you generally please attach patches to emails? That makes it much easier to ensure that one is looking at the same version and also to apply the patch to a local client.</div>
</blockquote><div><br></div><div>For reviews from me you can also make my life significantly easier (because I don't need to first patch it in locally) if you use <a href="http://llvm-reviews.chandlerc.com">http://llvm-reviews.chandlerc.com</a> (docs at <a href="http://llvm.org/docs/Phabricator.html">http://llvm.org/docs/Phabricator.html</a>).</div>
<div><br></div><div>Cheers,</div><div>/Manuel</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr"><div><br></div><div>I just have a few higher level comments and will leave comments to the parser part to Manuel.<br>
<div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Tue, Jan 14, 2014 at 9:32 PM, Diego Alexander Rojas <span dir="ltr"><<a href="mailto:alexander.rojas@gmail.com" target="_blank">alexander.rojas@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">I just completed the support for function-try blocks </blockquote>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>
--<br>
Diego Alexander Rojas Páez<br>
-------------- next part --------------<br>
Index: lib/Format/UnwrappedLineParser.cpp<br>
===================================================================<br>
--- lib/Format/UnwrappedLineParser.cpp (revision 199220)<br>
+++ lib/Format/UnwrappedLineParser.cpp (working copy)<br>
@@ -621,6 +621,9 @@<br>
case tok::kw_private:<br>
parseAccessSpecifier();<br>
return;<br>
+ case tok::kw_try:<br>
+ parseTryCatch();<br>
+ return;<br>
case tok::kw_if:<br>
parseIfThenElse();<br>
return;<br>
</div>@@ -708,6 +711,10 @@<br>
// Otherwise this was a braced init list, and the structural<br>
// element continues.<br>
break;<br>
+ case tok::kw_try:<br>
+ // It enters here in function-try blocks<br></blockquote><div><br></div></div></div><div>Maybe:</div><div>// This is a function try block.</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ parseTryCatch();<br>
+ return;<br>
case tok::identifier: {<br>
StringRef Text = FormatTok->TokenText;<br>
nextToken();<br>
@@ -1028,6 +1035,65 @@<br>
<div> }<br>
}<br>
<br>
+void UnwrappedLineParser::parseTryCatch() {<br>
+ assert(FormatTok->Tok.is(tok::kw_try) && "'try' expected");<br>
+ nextToken();<br>
+ bool NeedsUnwrappedLine = false;<br>
</div>+ if (FormatTok->Tok.is(tok::colon)) {<br>
+ // We are in a function try block, what comes is an initializer list<br></blockquote><div><br></div></div><div>Maybe:</div><div>// This starts the initializer list of a constructor try block.</div><div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ nextToken();<br>
+ while (FormatTok->Tok.is(tok::identifier)) {<br>
<div>+ nextToken();<br>
+ if (FormatTok->Tok.is(tok::l_paren))<br>
+ parseParens();<br>
</div>+ else<br>
+ StructuralError = true;<br>
+ if (FormatTok->Tok.is(tok::comma))<br>
+ nextToken();<br></blockquote><div><br></div></div><div>Preferably, we'd like test cases for all of the codepaths here.</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ }<br>
+ }<br>
<div><div>+ if (FormatTok->Tok.is(tok::l_brace)) {<br>
+ CompoundStatementIndenter Indenter(this, Style, Line->Level);<br>
+ parseBlock(/*MustBeDeclaration=*/false);<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman ||<br>
+ Style.BreakBeforeBraces == FormatStyle::BS_GNU ||<br>
+ Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup) {<br>
+ addUnwrappedLine();<br>
+ } else {<br>
+ NeedsUnwrappedLine = true;<br>
+ }<br>
+ } else if (!FormatTok->Tok.is(tok::kw_catch)) {<br>
+ // The C++ standard requires a compound-statement after a try.<br></div></div></blockquote><div><br></div></div><div>Do you mean "catch"-statement?</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>
+ // If there's none, we try to assume there's a structuralElement<br>
+ // and try to continue.<br>
+ StructuralError = true;<br>
+ addUnwrappedLine();<br>
+ ++Line->Level;<br>
+ parseStructuralElement();<br>
+ --Line->Level;<br>
+ }<br>
+ while (FormatTok->Tok.is(tok::kw_catch)) {<br>
+ nextToken();<br>
+ if (FormatTok->Tok.is(tok::l_paren))<br>
+ parseParens();<br>
+ NeedsUnwrappedLine = false;<br>
+ if (FormatTok->Tok.is(tok::l_brace)) {<br>
+ CompoundStatementIndenter Indenter(this, Style, Line->Level);<br>
+ parseBlock(/*MustBeDeclaration=*/false);<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Allman ||<br>
+ Style.BreakBeforeBraces == FormatStyle::BS_GNU ||<br>
+ Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup) {<br>
+ addUnwrappedLine();<br>
+ } else {<br>
+ NeedsUnwrappedLine = true;<br>
+ }<br>
+ }<br>
+ }<br>
+ if (NeedsUnwrappedLine) {<br>
+ addUnwrappedLine();<br>
+ }<br></div></div></blockquote><div><br></div></div></div><div>This needs a test case with multiple catch blocks.</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>
+}<br>
+<br>
void UnwrappedLineParser::parseNamespace() {<br>
assert(FormatTok->Tok.is(tok::kw_namespace) && "'namespace' expected");<br>
nextToken();<br>
Index: lib/Format/UnwrappedLineParser.h<br>
===================================================================<br>
--- lib/Format/UnwrappedLineParser.h (revision 199220)<br>
+++ lib/Format/UnwrappedLineParser.h (working copy)<br>
@@ -85,6 +85,7 @@<br>
void parseReturn();<br>
void parseParens();<br>
void parseSquare();<br>
+ void parseTryCatch();<br>
void parseIfThenElse();<br>
void parseForOrWhileLoop();<br>
void parseDoWhile();<br>
Index: unittests/Format/FormatTest.cpp<br>
===================================================================<br>
--- unittests/Format/FormatTest.cpp (revision 199220)<br>
+++ unittests/Format/FormatTest.cpp (working copy)<br>
@@ -1810,15 +1810,11 @@<br>
}<br>
<br>
TEST_F(FormatTest, FormatTryCatch) {<br>
- // FIXME: Handle try-catch explicitly in the UnwrappedLineParser, then we'll<br>
- // also not create single-line-blocks.<br>
verifyFormat("try {\n"<br>
" throw a * b;\n"<br>
- "}\n"<br>
- "catch (int a) {\n"<br>
+ "} catch (int a) {\n"<br>
" // Do nothing.\n"<br>
- "}\n"<br>
- "catch (...) {\n"<br>
+ "} catch (...) {\n"<br>
" exit(42);\n"<br>
"}");<br>
<br>
@@ -2271,9 +2267,8 @@<br>
" f(x)\n"<br>
" try {\n"<br>
" q();\n"<br>
+ " } catch (...) {\n"<br>
" }\n"<br>
- " catch (...) {\n"<br>
- " }\n"<br>
"}\n",<br>
format("int q() {\n"<br>
"f(x)\n"<br>
@@ -2288,8 +2283,7 @@<br>
EXPECT_EQ("class A {\n"<br>
" A() : t(0) {}\n"<br>
" A(X x)\n" // FIXME: function-level try blocks are broken.<br>
- " try : t(0) {\n"<br>
- " }\n"<br>
+ " try : t(0) {}\n"<br>
" catch (...) {\n"<br>
" }\n"<br>
"};",<br>
@@ -7179,9 +7173,8 @@<br>
TEST_F(FormatTest, CatchExceptionReferenceBinding) {<br>
verifyFormat("void f() {\n"<br>
" try {\n"<br>
+ " } catch (const Exception &e) {\n"<br>
" }\n"<br>
- " catch (const Exception &e) {\n"<br>
- " }\n"<br>
"}\n",<br>
getLLVMStyle());<span style="color:rgb(34,34,34)"> </span></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>
}<br></div></div></blockquote><div><br></div></div></div><div>We need tests for the different style options. Comment #10 on <a href="http://llvm.org/PR18418" target="_blank">llvm.org/PR18418</a> seems like a good set to start with.</div>
<div><br></div>
<div>Many thanks for working on this!</div><span class=""><font color="#888888"><div>Daniel</div></font></span><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>
_______________________________________________<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/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div></div><br></div></div></div></div>
</blockquote></div><br></div></div>