From 748e3668a8baee4c1b20c8e65b719f8bb886cf4f Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Mon, 20 Apr 2020 19:47:28 +0200 Subject: [PATCH 1/4] Support block comments --- src/sfizz/parser/Parser.cpp | 69 ++++++++++++++++++++++++++++++------- src/sfizz/parser/Parser.h | 4 +-- tests/DemoParser.cpp | 11 ++++++ 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/sfizz/parser/Parser.cpp b/src/sfizz/parser/Parser.cpp index eb3c496d7..800d0379f 100644 --- a/src/sfizz/parser/Parser.cpp +++ b/src/sfizz/parser/Parser.cpp @@ -113,7 +113,7 @@ void Parser::processTopLevel() while (!_included.empty()) { Reader& reader = *_included.back(); - while (reader.skipChars(" \t\r\n") || skipComment(reader)); + while (reader.skipChars(" \t\r\n") || skipComment()); switch (reader.peekChar()) { case Reader::kEof: @@ -348,32 +348,70 @@ void Parser::flushCurrentHeader() _currentOpcodes.clear(); } -bool Parser::hasComment(Reader& reader) +int Parser::hasComment(Reader& reader) { if (reader.peekChar() != '/') return false; reader.getChar(); - if (reader.peekChar() != '/') { - reader.putBackChar('/'); - return false; + + int ret = 0; + + switch (reader.peekChar()) { + case '/': + ret = 1; + break; + case '*': + ret = 2; + break; } - return true; + reader.putBackChar('/'); + return ret; } -size_t Parser::skipComment(Reader& reader) +size_t Parser::skipComment() { - if (!hasComment(reader)) + Reader& reader = *_included.back(); + + int commentType = hasComment(reader); + if (!commentType) return 0; + SourceLocation start = reader.location(); + size_t count = 2; reader.getChar(); reader.getChar(); - int c; - while ((c = reader.getChar()) != Reader::kEof && c != '\r' && c != '\n') - ++count; + bool terminated = false; + + switch (commentType) { + case 1: // line comment + { + int c; + while ((c = reader.getChar()) != Reader::kEof && c != '\r' && c != '\n') + ++count; + terminated = true; + } + break; + case 2: // block comment + { + int c1 = 0; + int c2 = reader.getChar(); + while (!terminated && c2 != Reader::kEof) { + c1 = c2; + c2 = reader.getChar(); + terminated = c1 == '*' && c2 == '/'; + } + } + break; + } + + if (!terminated) { + SourceLocation end = reader.location(); + emitError({ start, end }, "Unterminated block comment."); + } return count; } @@ -387,7 +425,14 @@ void Parser::trimRight(std::string& text) size_t Parser::extractToEol(Reader& reader, std::string* dst) { return reader.extractWhile(dst, [&reader](char c) { - return c != '\r' && c != '\n' && !(c == '/' && reader.peekChar() == '/'); + if (c == '\r' || c == '\n') + return false; + if (c == '/') { + int c2 = reader.peekChar(); + if (c2 == '/' || c2 == '*') // stop at comment + return false; + } + return true; }); } diff --git a/src/sfizz/parser/Parser.h b/src/sfizz/parser/Parser.h index 4f6a071d6..88424d1d9 100644 --- a/src/sfizz/parser/Parser.h +++ b/src/sfizz/parser/Parser.h @@ -84,8 +84,8 @@ class Parser { void flushCurrentHeader(); // helpers - static bool hasComment(Reader& reader); - static size_t skipComment(Reader& reader); + static int hasComment(Reader& reader); // 0=None 1=Line 2=Block + size_t skipComment(); static void trimRight(std::string& text); static size_t extractToEol(Reader& reader, std::string* dst); // ignores comment std::string expandDollarVars(const SourceRange& range, absl::string_view src); diff --git a/tests/DemoParser.cpp b/tests/DemoParser.cpp index f7931694e..424dcceea 100644 --- a/tests/DemoParser.cpp +++ b/tests/DemoParser.cpp @@ -38,6 +38,11 @@ static const char defaultSfzText[] = R"SFZ( // This is a SFZ test file with many problems. // //----------------------------------------------------------------------------// +/* + * This is a block comment. Not all the SFZ players accept it. + * It can span over multiple lines. +*/ + // opcode without header not_in_header=on // warning @@ -75,6 +80,12 @@ abcdef=$tata // opcode name which expands to invalid identifier $titi=1 + +volume=10 /* +block comments at the end of line +*/ + +/* unterminated block comment )SFZ"; void Application::init() From ea425f380c5d1f45fd6d6df1bbe7cc6276542e9b Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Mon, 20 Apr 2020 20:12:38 +0200 Subject: [PATCH 2/4] Add some parsing tests --- tests/ParsingT.cpp | 75 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tests/ParsingT.cpp b/tests/ParsingT.cpp index 7a51263c5..840315863 100644 --- a/tests/ParsingT.cpp +++ b/tests/ParsingT.cpp @@ -448,3 +448,78 @@ param2=$bar)"); REQUIRE(mock.fullBlockHeaders == expectedHeaders); REQUIRE(mock.fullBlockMembers == expectedMembers); } + +TEST_CASE("[Parsing] Block comments") +{ + sfz::Parser parser; + ParsingMocker mock; + parser.setListener(&mock); + parser.parseString("/blockComments.sfz", +R"(/* A block comment (1) */ +/* +A block comment (2) */ +/* A block comment (3) +*/ +/* A block comment + (4) */ +/* A block comment /* // ** (5) */ +)"); + REQUIRE(mock.beginnings == 1); + REQUIRE(mock.endings == 1); + REQUIRE(mock.errors.empty()); + REQUIRE(mock.warnings.empty()); + REQUIRE(mock.opcodes.empty()); + REQUIRE(mock.headers.empty()); + REQUIRE(mock.fullBlockHeaders.empty()); + REQUIRE(mock.fullBlockMembers.empty()); +} + +TEST_CASE("[Parsing] Unterminated block comments") +{ + sfz::Parser parser; + ParsingMocker mock; + parser.setListener(&mock); + parser.parseString("/unterminatedBlockComment.sfz", +R"(/* Unterminated block comment +)"); + REQUIRE(mock.beginnings == 1); + REQUIRE(mock.endings == 1); + REQUIRE(mock.errors.size() == 1); + REQUIRE(mock.warnings.empty()); + REQUIRE(mock.opcodes.empty()); + REQUIRE(mock.headers.empty()); + REQUIRE(mock.fullBlockHeaders.empty()); + REQUIRE(mock.fullBlockMembers.empty()); +} + +TEST_CASE("[Parsing] Comments after values") +{ + sfz::Parser parser; + ParsingMocker mock; + parser.setListener(&mock); + parser.parseString("/commentsAfterValues.sfz", +R"(
+param1=foo param2=bar // line comment +param3=baz param4=quux /* block comment */)"); + std::vector> expectedMembers = { + {{"param1", "foo"}, {"param2", "bar"}, + {"param3", "baz"}, {"param4", "quux"}} + }; + std::vector expectedHeaders = { + "header" + }; + std::vector expectedOpcodes; + + for (auto& members: expectedMembers) + for (auto& opcode: members) + expectedOpcodes.push_back(opcode); + + REQUIRE(mock.beginnings == 1); + REQUIRE(mock.endings == 1); + REQUIRE(mock.errors.empty()); + REQUIRE(mock.warnings.empty()); + REQUIRE(mock.opcodes == expectedOpcodes); + REQUIRE(mock.headers == expectedHeaders); + REQUIRE(mock.fullBlockHeaders == expectedHeaders); + REQUIRE(mock.fullBlockMembers == expectedMembers); +} From 605e552a251a5a05224195608b75e668d6bbd190 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Mon, 20 Apr 2020 22:54:04 +0200 Subject: [PATCH 3/4] Change the comment type into an enum --- src/sfizz/parser/Parser.cpp | 22 +++++++++++++--------- src/sfizz/parser/Parser.h | 8 +++++++- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/sfizz/parser/Parser.cpp b/src/sfizz/parser/Parser.cpp index 800d0379f..ffa660aa7 100644 --- a/src/sfizz/parser/Parser.cpp +++ b/src/sfizz/parser/Parser.cpp @@ -7,6 +7,7 @@ #include "Parser.h" #include "ParserPrivate.h" #include "absl/memory/memory.h" +#include namespace sfz { @@ -348,21 +349,21 @@ void Parser::flushCurrentHeader() _currentOpcodes.clear(); } -int Parser::hasComment(Reader& reader) +Parser::CommentType Parser::getCommentType(Reader& reader) { if (reader.peekChar() != '/') - return false; + return CommentType::None; reader.getChar(); - int ret = 0; + CommentType ret = CommentType::None; switch (reader.peekChar()) { case '/': - ret = 1; + ret = CommentType::Line; break; case '*': - ret = 2; + ret = CommentType::Block; break; } @@ -374,8 +375,8 @@ size_t Parser::skipComment() { Reader& reader = *_included.back(); - int commentType = hasComment(reader); - if (!commentType) + const CommentType commentType = getCommentType(reader); + if (commentType == CommentType::None) return 0; SourceLocation start = reader.location(); @@ -387,7 +388,7 @@ size_t Parser::skipComment() bool terminated = false; switch (commentType) { - case 1: // line comment + case CommentType::Line: { int c; while ((c = reader.getChar()) != Reader::kEof && c != '\r' && c != '\n') @@ -395,7 +396,7 @@ size_t Parser::skipComment() terminated = true; } break; - case 2: // block comment + case CommentType::Block: { int c1 = 0; int c2 = reader.getChar(); @@ -406,6 +407,9 @@ size_t Parser::skipComment() } } break; + default: + assert(false); + break; } if (!terminated) { diff --git a/src/sfizz/parser/Parser.h b/src/sfizz/parser/Parser.h index 88424d1d9..057143ad6 100644 --- a/src/sfizz/parser/Parser.h +++ b/src/sfizz/parser/Parser.h @@ -84,7 +84,13 @@ class Parser { void flushCurrentHeader(); // helpers - static int hasComment(Reader& reader); // 0=None 1=Line 2=Block + enum class CommentType { + None, + Line, + Block, + }; + + static CommentType getCommentType(Reader& reader); size_t skipComment(); static void trimRight(std::string& text); static size_t extractToEol(Reader& reader, std::string* dst); // ignores comment From 353cea949e8490c358f03d981949a8e074020e1a Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Mon, 20 Apr 2020 22:59:00 +0200 Subject: [PATCH 4/4] Make the skip count correct (even if not used currently) --- src/sfizz/parser/Parser.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/sfizz/parser/Parser.cpp b/src/sfizz/parser/Parser.cpp index ffa660aa7..455c0aec5 100644 --- a/src/sfizz/parser/Parser.cpp +++ b/src/sfizz/parser/Parser.cpp @@ -389,20 +389,21 @@ size_t Parser::skipComment() switch (commentType) { case CommentType::Line: - { - int c; - while ((c = reader.getChar()) != Reader::kEof && c != '\r' && c != '\n') - ++count; - terminated = true; + while (!terminated) { + int c = reader.getChar(); + count += (c != Reader::kEof); + terminated = c == Reader::kEof || c == '\r' || c == '\n'; } break; case CommentType::Block: { int c1 = 0; int c2 = reader.getChar(); + count += (c2 != Reader::kEof); while (!terminated && c2 != Reader::kEof) { c1 = c2; c2 = reader.getChar(); + count += (c2 != Reader::kEof); terminated = c1 == '*' && c2 == '/'; } }