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.
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
