On Tue, Jan 14, 2014 at 9:53 PM, Daniel Jasper <[email protected]> wrote:
> 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. > 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 http://llvm-reviews.chandlerc.com (docs at http://llvm.org/docs/Phabricator.html). Cheers, /Manuel > > I just have a few higher level comments and will leave comments to the > parser part to Manuel. > > On Tue, Jan 14, 2014 at 9:32 PM, Diego Alexander Rojas < > [email protected]> wrote: > >> I just completed the support for function-try blocks > > >> -- >> Diego Alexander Rojas Páez >> -------------- next part -------------- >> Index: lib/Format/UnwrappedLineParser.cpp >> =================================================================== >> --- lib/Format/UnwrappedLineParser.cpp (revision 199220) >> +++ lib/Format/UnwrappedLineParser.cpp (working copy) >> @@ -621,6 +621,9 @@ >> case tok::kw_private: >> parseAccessSpecifier(); >> return; >> + case tok::kw_try: >> + parseTryCatch(); >> + return; >> case tok::kw_if: >> parseIfThenElse(); >> return; >> @@ -708,6 +711,10 @@ >> // Otherwise this was a braced init list, and the structural >> // element continues. >> break; >> + case tok::kw_try: >> + // It enters here in function-try blocks >> > > Maybe: > // This is a function try block. > > >> + parseTryCatch(); >> + return; >> case tok::identifier: { >> StringRef Text = FormatTok->TokenText; >> nextToken(); >> @@ -1028,6 +1035,65 @@ >> } >> } >> >> +void UnwrappedLineParser::parseTryCatch() { >> + assert(FormatTok->Tok.is(tok::kw_try) && "'try' expected"); >> + nextToken(); >> + bool NeedsUnwrappedLine = false; >> + if (FormatTok->Tok.is(tok::colon)) { >> + // We are in a function try block, what comes is an initializer >> list >> > > Maybe: > // This starts the initializer list of a constructor try block. > > >> + nextToken(); >> + while (FormatTok->Tok.is(tok::identifier)) { >> + nextToken(); >> + if (FormatTok->Tok.is(tok::l_paren)) >> + parseParens(); >> + else >> + StructuralError = true; >> + if (FormatTok->Tok.is(tok::comma)) >> + nextToken(); >> > > Preferably, we'd like test cases for all of the codepaths here. > > >> + } >> + } >> + if (FormatTok->Tok.is(tok::l_brace)) { >> + CompoundStatementIndenter Indenter(this, Style, Line->Level); >> + parseBlock(/*MustBeDeclaration=*/false); >> + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman || >> + Style.BreakBeforeBraces == FormatStyle::BS_GNU || >> + Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup) { >> + addUnwrappedLine(); >> + } else { >> + NeedsUnwrappedLine = true; >> + } >> + } else if (!FormatTok->Tok.is(tok::kw_catch)) { >> + // The C++ standard requires a compound-statement after a try. >> > > Do you mean "catch"-statement? > > >> + // If there's none, we try to assume there's a structuralElement >> + // and try to continue. >> + StructuralError = true; >> + addUnwrappedLine(); >> + ++Line->Level; >> + parseStructuralElement(); >> + --Line->Level; >> + } >> + while (FormatTok->Tok.is(tok::kw_catch)) { >> + nextToken(); >> + if (FormatTok->Tok.is(tok::l_paren)) >> + parseParens(); >> + NeedsUnwrappedLine = false; >> + if (FormatTok->Tok.is(tok::l_brace)) { >> + CompoundStatementIndenter Indenter(this, Style, Line->Level); >> + parseBlock(/*MustBeDeclaration=*/false); >> + if (Style.BreakBeforeBraces == FormatStyle::BS_Allman || >> + Style.BreakBeforeBraces == FormatStyle::BS_GNU || >> + Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup) { >> + addUnwrappedLine(); >> + } else { >> + NeedsUnwrappedLine = true; >> + } >> + } >> + } >> + if (NeedsUnwrappedLine) { >> + addUnwrappedLine(); >> + } >> > > This needs a test case with multiple catch blocks. > > >> +} >> + >> void UnwrappedLineParser::parseNamespace() { >> assert(FormatTok->Tok.is(tok::kw_namespace) && "'namespace' expected"); >> nextToken(); >> Index: lib/Format/UnwrappedLineParser.h >> =================================================================== >> --- lib/Format/UnwrappedLineParser.h (revision 199220) >> +++ lib/Format/UnwrappedLineParser.h (working copy) >> @@ -85,6 +85,7 @@ >> void parseReturn(); >> void parseParens(); >> void parseSquare(); >> + void parseTryCatch(); >> void parseIfThenElse(); >> void parseForOrWhileLoop(); >> void parseDoWhile(); >> Index: unittests/Format/FormatTest.cpp >> =================================================================== >> --- unittests/Format/FormatTest.cpp (revision 199220) >> +++ unittests/Format/FormatTest.cpp (working copy) >> @@ -1810,15 +1810,11 @@ >> } >> >> TEST_F(FormatTest, FormatTryCatch) { >> - // FIXME: Handle try-catch explicitly in the UnwrappedLineParser, then >> we'll >> - // also not create single-line-blocks. >> verifyFormat("try {\n" >> " throw a * b;\n" >> - "}\n" >> - "catch (int a) {\n" >> + "} catch (int a) {\n" >> " // Do nothing.\n" >> - "}\n" >> - "catch (...) {\n" >> + "} catch (...) {\n" >> " exit(42);\n" >> "}"); >> >> @@ -2271,9 +2267,8 @@ >> " f(x)\n" >> " try {\n" >> " q();\n" >> + " } catch (...) {\n" >> " }\n" >> - " catch (...) {\n" >> - " }\n" >> "}\n", >> format("int q() {\n" >> "f(x)\n" >> @@ -2288,8 +2283,7 @@ >> EXPECT_EQ("class A {\n" >> " A() : t(0) {}\n" >> " A(X x)\n" // FIXME: function-level try blocks are broken. >> - " try : t(0) {\n" >> - " }\n" >> + " try : t(0) {}\n" >> " catch (...) {\n" >> " }\n" >> "};", >> @@ -7179,9 +7173,8 @@ >> TEST_F(FormatTest, CatchExceptionReferenceBinding) { >> verifyFormat("void f() {\n" >> " try {\n" >> + " } catch (const Exception &e) {\n" >> " }\n" >> - " catch (const Exception &e) {\n" >> - " }\n" >> "}\n", >> getLLVMStyle()); >> > } >> > > We need tests for the different style options. Comment #10 on > llvm.org/PR18418 seems like a good set to start with. > > Many thanks for working on this! > Daniel > > _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
