grame-cncm / faust

Functional programming language for signal processing and sound synthesis
http://faust.grame.fr
Other
2.58k stars 322 forks source link

"mydsp" substitution behavior in architecture files changed? #229

Closed PaulBatchelor closed 6 years ago

PaulBatchelor commented 6 years ago

Just recompiled FAUST today, and it seems like the substitution behavior in the architecture files has been broken.

When I run the following command:

faust -lang c -a minimal.c -cn zebra foo.dsp

The generated output does not replace all the instances of "mydsp" with "zebra". The minimal template used above makes calls to functions like newmydsp and deletemydsp when the functions should have been called newzebra and deletezebra. The faust generated code generates the name changes just fine, so the correct named functions do exist.

I am positive this is a change in behavior from previous builds of faust that I've used because I have custom architecture files that used this feature.

The files used to produce this behavior can be found below.

files.zip

PaulBatchelor commented 6 years ago

This bug only happens on master-dev. On master, things behave as expected.

sletz commented 6 years ago

@orlarey ping: this is related to this commit https://github.com/grame-cncm/faust/commit/fba792c65789170b35cee45aefe01b54dd3bc804 and the addition of the wordBoundaries check The strange thing is that without any fix,./testsuccessrenamed already failed on some faust2xxx scripts, is it supposed to happen ?

OK: faust2android  succeeded !
OK: faust2android -cn Bidule succeeded !
ERROR: faust2android -cn Bidule -scn Machin failed
OK: faust2au  succeeded !
OK: faust2au -cn Bidule succeeded !
ERROR: faust2au -cn Bidule -scn Machin failed

And a possible fix could be :

--- a/compiler/parser/enrobage.cpp
+++ b/compiler/parser/enrobage.cpp
@@ -61,14 +61,14 @@ static bool wordBoundaries(const string& str, string::size_type pos, string::siz
  * Replace every occurrence of oldstr by newstr inside str. str is modified
  * and returned as reference for convenience
  */
-static string& replaceOccurrences(string& str, const string& oldstr, const string& newstr)
+static string& replaceOccurrences(string& str, const string& oldstr, const string& newstr, bool force)
 {
     string::size_type l1 = oldstr.length();
     string::size_type l2 = newstr.length();

     string::size_type pos = str.find(oldstr);
     while (pos != string::npos) {
-        if (wordBoundaries(str, pos, l1)) {
+        if (force || wordBoundaries(str, pos, l1)) {
             // cerr << "'" << str << "'@" << pos << " replace '" << oldstr << "' by '" << newstr << "'" << endl;
             str.replace(pos, l1, newstr);
             pos = str.find(oldstr, pos + l2);
@@ -87,8 +87,8 @@ static string& replaceOccurrences(string& str, const string& oldstr, const strin
 static string& replaceClassName(string& str)
 {
     string res;
-    replaceOccurrences(str, "mydsp", gGlobal->gClassName);
-    replaceOccurrences(str, "dsp", gGlobal->gSuperClassName);
+    replaceOccurrences(str, "mydsp", gGlobal->gClassName, true);
+    replaceOccurrences(str, "dsp", gGlobal->gSuperClassName, false);
     return str;
 }

Can you check ? thanks.

PaulBatchelor commented 6 years ago

Thank you for the fast response!

I tried to apply this patch with:

patch -p1 < fix.patch

But I get this error:

patching file compiler/parser/enrobage.cpp patch unexpectedly ends in middle of line Hunk #2 succeeded at 87 with fuzz 2.

Am I doing something wrong here? Attaching the patch I used, copied and pasted from this issue. fix.patch.zip

sletz commented 6 years ago

I would prefer @orlarey to have a look before. It this patch solves the issue, then we'll push it on githuh.

orlarey commented 6 years ago

OK @sletz I'll have a look...

sletz commented 6 years ago

Fixed in https://github.com/grame-cncm/faust/commit/e4f355e6a1aeed8272100961c87a2e0e08766955.

PaulBatchelor commented 6 years ago

Confirmed. You folks are fast. Thanks again!