From 650c6140bd92e366734f5bcb8e33feea82309115 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Thu, 14 May 2020 20:48:08 +0200 Subject: [PATCH 1/5] parser: dollar expansions and multiple #define on the same line --- src/sfizz/parser/Parser.cpp | 23 ++++++++++--- tests/FilesT.cpp | 10 +++++- tests/ParsingT.cpp | 66 +++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/src/sfizz/parser/Parser.cpp b/src/sfizz/parser/Parser.cpp index 455c0aec..2c349e20 100644 --- a/src/sfizz/parser/Parser.cpp +++ b/src/sfizz/parser/Parser.cpp @@ -163,7 +163,18 @@ void Parser::processDirective() std::string value; extractToEol(reader, &value); + +#if 1 + // ARIA/not Cakewalk: cut the value after the first word + size_t position = value.find_first_of(" \t"); + if (position != value.npos) { + absl::string_view excess(&value[position], value.size() - position); + reader.putBackChars(excess); + value.resize(position); + } +#else trimRight(value); +#endif addDefinition(id, value); } @@ -457,21 +468,25 @@ std::string Parser::expandDollarVars(const SourceRange& range, absl::string_view std::string name; name.reserve(64); - while (i < n && isIdentifierChar(src[i])) + // ARIA: we will accumulate any chars after $, until this is the + // name of a known variable + auto def = _currentDefinitions.end(); + while (i < n && isIdentifierChar(src[i]) && def == _currentDefinitions.end()) { name.push_back(src[i++]); + def = _currentDefinitions.find(name); + } if (name.empty()) { emitWarning(range, "Expected variable name after $."); continue; } - auto it = _currentDefinitions.find(name); - if (it == _currentDefinitions.end()) { + if (def == _currentDefinitions.end()) { emitWarning(range, "The variable `" + name + "` is not defined."); continue; } - dst.append(it->second); + dst.append(def->second); } } diff --git a/tests/FilesT.cpp b/tests/FilesT.cpp index dbfaea38..0ae8f068 100644 --- a/tests/FilesT.cpp +++ b/tests/FilesT.cpp @@ -339,14 +339,22 @@ TEST_CASE("[Files] sw_default and playing with switches") REQUIRE( synth.getRegionView(3)->isSwitchedOn() ); } - TEST_CASE("[Files] wrong (overlapping) replacement for defines") { sfz::Synth synth; synth.loadSfzFile(fs::current_path() / "tests/TestFiles/SpecificBugs/wrong-replacements.sfz"); + REQUIRE( synth.getNumRegions() == 3 ); + +#if 0 + // Note: test checked to be wrong under Sforzando 1.961 + // It is the shorter matching $-variable which matches among both. + // The rest of the variable name creates some trailing junk text + // which Sforzando accepts without warning. (eg. `key=52Edge`) REQUIRE( synth.getRegionView(0)->keyRange.getStart() == 52 ); REQUIRE( synth.getRegionView(0)->keyRange.getEnd() == 52 ); +#endif + REQUIRE( synth.getRegionView(1)->keyRange.getStart() == 57 ); REQUIRE( synth.getRegionView(1)->keyRange.getEnd() == 57 ); REQUIRE(!synth.getRegionView(2)->amplitudeCC.empty()); diff --git a/tests/ParsingT.cpp b/tests/ParsingT.cpp index 84031586..b259e4a7 100644 --- a/tests/ParsingT.cpp +++ b/tests/ParsingT.cpp @@ -523,3 +523,69 @@ param3=baz param4=quux /* block comment */)"); REQUIRE(mock.fullBlockHeaders == expectedHeaders); REQUIRE(mock.fullBlockMembers == expectedMembers); } + +TEST_CASE("[Parsing] Overlapping definition identifiers") +{ + sfz::Parser parser; + ParsingMocker mock; + parser.setListener(&mock); + parser.parseString("/overlappingDefinitionIdentifiers.sfz", +R"(#define $abc foo +#define $abcdef bar + sample=$abc.wav + sample=$abcdef.wav)"); + + std::vector> expectedMembers = { + {{"sample", "foo.wav"}}, + {{"sample", "foodef.wav"}}, + }; + std::vector expectedHeaders = { + "region", "region" + }; + 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); +} + +TEST_CASE("[Parsing] Interpretation of the value of #define") +{ + sfz::Parser parser; + ParsingMocker mock; + parser.setListener(&mock); + parser.parseString("/defineValues.sfz", +R"(#define $a foo #define $b bar sample=$a-$b.wav +#define $c toto sample=$c.wav)"); + + std::vector> expectedMembers = { + {{"sample", "foo-bar.wav"}}, + {{"sample", "toto.wav"}}, + }; + std::vector expectedHeaders = { + "region", "region" + }; + 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 0c6bf5a7a97d2a1b9403506dd12226bfc68af461 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Thu, 14 May 2020 21:24:24 +0200 Subject: [PATCH 2/5] parser: support recursive $-expansions --- src/sfizz/parser/Parser.cpp | 67 +++++++++++++++++++++++-------------- tests/ParsingT.cpp | 32 ++++++++++++++++++ 2 files changed, 73 insertions(+), 26 deletions(-) diff --git a/src/sfizz/parser/Parser.cpp b/src/sfizz/parser/Parser.cpp index 2c349e20..51eab71e 100644 --- a/src/sfizz/parser/Parser.cpp +++ b/src/sfizz/parser/Parser.cpp @@ -455,38 +455,53 @@ size_t Parser::extractToEol(Reader& reader, std::string* dst) std::string Parser::expandDollarVars(const SourceRange& range, absl::string_view src) { std::string dst; + std::string srcbuf; // temporary for retries when recursive + std::string name; // temporary for variable name + bool keepExpanding = true; + dst.reserve(2 * src.size()); + name.reserve(64); - size_t i = 0; - size_t n = src.size(); - while (i < n) { - char c = src[i++]; + while (keepExpanding) { + size_t i = 0; + size_t n = src.size(); + size_t numExpansions = 0; + while (i < n) { + char c = src[i++]; - if (c != '$') - dst.push_back(c); - else { - std::string name; - name.reserve(64); + if (c != '$') + dst.push_back(c); + else { + ++numExpansions; + name.clear(); - // ARIA: we will accumulate any chars after $, until this is the - // name of a known variable - auto def = _currentDefinitions.end(); - while (i < n && isIdentifierChar(src[i]) && def == _currentDefinitions.end()) { - name.push_back(src[i++]); - def = _currentDefinitions.find(name); + // ARIA: we will accumulate any chars after $, until this is the + // name of a known variable + auto def = _currentDefinitions.end(); + while (i < n && isIdentifierChar(src[i]) && def == _currentDefinitions.end()) { + name.push_back(src[i++]); + def = _currentDefinitions.find(name); + } + + if (name.empty()) { + emitWarning(range, "Expected variable name after $."); + continue; + } + + if (def == _currentDefinitions.end()) { + emitWarning(range, "The variable `" + name + "` is not defined."); + continue; + } + + dst.append(def->second); } + } - if (name.empty()) { - emitWarning(range, "Expected variable name after $."); - continue; - } - - if (def == _currentDefinitions.end()) { - emitWarning(range, "The variable `" + name + "` is not defined."); - continue; - } - - dst.append(def->second); + keepExpanding = numExpansions > 0; + if (keepExpanding) { + srcbuf = dst; + src = srcbuf; + dst.clear(); } } diff --git a/tests/ParsingT.cpp b/tests/ParsingT.cpp index b259e4a7..2c4127d5 100644 --- a/tests/ParsingT.cpp +++ b/tests/ParsingT.cpp @@ -589,3 +589,35 @@ R"(#define $a foo #define $b bar sample=$a-$b.wav REQUIRE(mock.fullBlockHeaders == expectedHeaders); REQUIRE(mock.fullBlockMembers == expectedMembers); } + +TEST_CASE("[Parsing] Recursive expansion") +{ + sfz::Parser parser; + ParsingMocker mock; + parser.setListener(&mock); + parser.parseString("/recursiveExpansion.sfz", +R"(#define $B foo-$A-baz +#define $A bar + sample=$B.wav)"); + + std::vector> expectedMembers = { + {{"sample", "foo-bar-baz.wav"}}, + }; + std::vector expectedHeaders = { + "region" + }; + 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 a138b93182638e1748e91c3d46f4995abe04ec11 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Fri, 15 May 2020 09:15:58 +0200 Subject: [PATCH 3/5] Test against a regression with accepted filenames --- tests/ParsingT.cpp | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/ParsingT.cpp b/tests/ParsingT.cpp index 2c4127d5..3249ac31 100644 --- a/tests/ParsingT.cpp +++ b/tests/ParsingT.cpp @@ -621,3 +621,34 @@ R"(#define $B foo-$A-baz REQUIRE(mock.fullBlockHeaders == expectedHeaders); REQUIRE(mock.fullBlockMembers == expectedMembers); } + +TEST_CASE("[Parsing] Opcode value special character") +{ + sfz::Parser parser; + ParsingMocker mock; + parser.setListener(&mock); + parser.parseString("/opcodeValueSpecialCharacter.sfz", +R"( +sample=Alto-Flute-sus-C#4-PB-loop.wav)"); + + std::vector> expectedMembers = { + {{"sample", "Alto-Flute-sus-C#4-PB-loop.wav"}}, + }; + std::vector expectedHeaders = { + "region" + }; + 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 4b8eaebdc51d0562267d30d0bac8fa0f18bffff0 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Fri, 15 May 2020 14:55:14 +0200 Subject: [PATCH 4/5] Fix some cases of value parsing under ARIA (fix Salamander) --- src/sfizz/parser/Parser.cpp | 59 ++++++++++++++++++++++--------------- tests/ParsingT.cpp | 35 +++++++++++++++++++++- 2 files changed, 69 insertions(+), 25 deletions(-) diff --git a/src/sfizz/parser/Parser.cpp b/src/sfizz/parser/Parser.cpp index 51eab71e..a08d2bcc 100644 --- a/src/sfizz/parser/Parser.cpp +++ b/src/sfizz/parser/Parser.cpp @@ -285,36 +285,47 @@ void Parser::processOpcode() std::string valueRaw; extractToEol(reader, &valueRaw); - // if a "=" or "<" character was hit, it means we read too far - size_t position = valueRaw.find_first_of("=<"); - if (position != valueRaw.npos) { - char hitChar = valueRaw[position]; + size_t endPosition = 0; - // if it was "=", rewind before the opcode name and spaces preceding - if (hitChar == '=') { - while (position > 0 && isRawOpcodeNameChar(valueRaw[position - 1])) - --position; - while (position > 0 && isSpaceChar(valueRaw[position - 1])) - --position; + for (size_t valueSize = valueRaw.size(); endPosition < valueSize;) { + size_t i = endPosition + 1; + + if (isSpaceChar(valueRaw[endPosition])) { + // check if the rest of the string is to consume or not + bool stop = false; + + // consume space characters following + while (i < valueSize && isSpaceChar(valueRaw[endPosition + 1])) + ++i; + + // if there aren't non-space characters following, do not extract + if (i == valueSize) + stop = true; + // if a "=" or "<" character is next, a header or a directive follows + else if (valueRaw[i] == '<' || valueRaw[i] == '#') + stop = true; + // if sequence of identifier chars and then "=", an opcode follows + else if (isIdentifierChar(valueRaw[i])) { + ++i; + while (i < valueSize && isIdentifierChar(valueRaw[i])) + ++i; + if (i < valueSize && valueRaw[i] == '=') + stop = true; + } + + if (stop) + break; } - absl::string_view excess(&valueRaw[position], valueRaw.size() - position); + endPosition = i; + } + + if (endPosition != valueRaw.size()) { + absl::string_view excess(&valueRaw[endPosition], valueRaw.size() - endPosition); reader.putBackChars(excess); - valueRaw.resize(position); - - // ensure that we are landing back next to a space char - if (hitChar == '=' && !reader.hasOneOfChars(" \t\r\n")) { - SourceLocation end = reader.location(); - emitError({ valueStart, end }, "Unexpected `=` in opcode value."); - recover(); - return; - } + valueRaw.resize(endPosition); } - while (!valueRaw.empty() && isSpaceChar(valueRaw.back())) { - reader.putBackChar(valueRaw.back()); - valueRaw.pop_back(); - } SourceLocation valueEnd = reader.location(); if (!_currentHeader) diff --git a/tests/ParsingT.cpp b/tests/ParsingT.cpp index 3249ac31..b54b40db 100644 --- a/tests/ParsingT.cpp +++ b/tests/ParsingT.cpp @@ -629,10 +629,43 @@ TEST_CASE("[Parsing] Opcode value special character") parser.setListener(&mock); parser.parseString("/opcodeValueSpecialCharacter.sfz", R"( -sample=Alto-Flute-sus-C#4-PB-loop.wav)"); +sample=Alto-Flute-sus-C#4-PB-loop.wav + +sample=foo=bar> expectedMembers = { {{"sample", "Alto-Flute-sus-C#4-PB-loop.wav"}}, + {{"sample", "foo=bar expectedHeaders = { + "region", "region" + }; + 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); +} + +TEST_CASE("[Parsing] Opcode value with inline directives") +{ + sfz::Parser parser; + ParsingMocker mock; + parser.setListener(&mock); + parser.parseString("/opcodeValueWithInlineDirective.sfz", +R"(#define $VEL v1 sample=$VEL.wav #define $FOO bar)"); + + std::vector> expectedMembers = { + {{"sample", "v1.wav"}}, }; std::vector expectedHeaders = { "region" From 8a17e18e9f562bb79f7dc1bc7a007b94a90b916d Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Sat, 16 May 2020 18:03:04 +0200 Subject: [PATCH 5/5] Fix an error with multiple consecutive space --- src/sfizz/parser/Parser.cpp | 2 +- tests/ParsingT.cpp | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/sfizz/parser/Parser.cpp b/src/sfizz/parser/Parser.cpp index a08d2bcc..a5ebd57f 100644 --- a/src/sfizz/parser/Parser.cpp +++ b/src/sfizz/parser/Parser.cpp @@ -295,7 +295,7 @@ void Parser::processOpcode() bool stop = false; // consume space characters following - while (i < valueSize && isSpaceChar(valueRaw[endPosition + 1])) + while (i < valueSize && isSpaceChar(valueRaw[i])) ++i; // if there aren't non-space characters following, do not extract diff --git a/tests/ParsingT.cpp b/tests/ParsingT.cpp index b54b40db..41fe8383 100644 --- a/tests/ParsingT.cpp +++ b/tests/ParsingT.cpp @@ -685,3 +685,34 @@ R"(#define $VEL v1 sample=$VEL.wav #define $FOO bar)"); REQUIRE(mock.fullBlockHeaders == expectedHeaders); REQUIRE(mock.fullBlockMembers == expectedMembers); } + +TEST_CASE("[Parsing] Opcode value with multiple consecutive spaces") +{ + sfz::Parser parser; + ParsingMocker mock; + parser.setListener(&mock); + parser.parseString("/opcodeValueWithMultipleConsecutiveSpaces.sfz", +R"( sample=foo bar baz .wav key=69 )"); + + std::vector> expectedMembers = { + {{"sample", "foo bar baz .wav"}, + {"key", "69"}}, + }; + std::vector expectedHeaders = { + "region" + }; + 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); +}