Compilation errors in SoloCup Release

[kgracey]

@zfi couldn't find the orange Beta label.

Many compilation errors. Load and check. Two files attached

Pixy2_X-Y-Axis_Roaming Working Get Object.zip

[zfi]

It looks like the the code that is rendering constants is missing a CR/LF. The ab_360 library is not getting included.

v1.4.2 source:

// ------ Libraries and Definitions ------
#include "simpletools.h"
#define MY_SIG_1 1
#define MY_SIG_2 2
#define MY_SIG_3 4
#define MY_SIG_4 8
#define MY_SIG_5 16
#define MY_SIG_6 32
#define MY_SIG_7 64
#define MY_SIG_ALL 127
#define MY_COLOR_CODES 128
#include "abdrive360.h"
#include "fdserial.h"
#include "servo.h"
[...]

v.1.5.0-b1 source:

// ------ Libraries and Definitions ------
#include "simpletools.h"

#define MY_SIG_1 1
#define MY_SIG_2 2
#define MY_SIG_3 4
#define MY_SIG_4 8
#define MY_SIG_5 16
#define MY_SIG_6 32
#define MY_SIG_7 64
#define MY_SIG_ALL 127
#define MY_COLOR_CODES 128#include "abdrive360.h"

#include "fdserial.h"

#include "servo.h"

[zfi]

Confirmed. This two block project generates the exact same behavior.

// ------ Libraries and Definitions ------
#include "simpletools.h"

#define MY_MYVALUE 128#include "fdserial.h"

// ------ Global Variables and Objects ------
fdserial *fdser31_30;

// ------ Main Program ------
int main() {
  fdser31_30 = fdserial_open(31, 30, 0b0000, 2400);

}

[PropGit]

This appears to represent a non-systemic problem in the new code; ie: an isolated problem. Am I right? I'm asking to get a feel for how much this fix will "fix" and gauge how many more unrelated problems we may find/look for.

[zfi]

Correct. This looks like the code generator that assembles all of the constant declarations near the top of the file is now omitting a CR/LF at the end of that section. Also the next section appears to have an extra CR/LF for each #include declaration, which might be a clue as to what is really broken.

[zfi]

If the blocks are swapped such that the constants appear after the init block, the code works.

The code output from Blockly.propc.finish()

"

// ------ Libraries and Definitions ------
#include "simpletools.h"
#include "fdserial.h"
#define MY_MYVALUE  128

// ------ Global Variables and Objects ------
fdserial *fdser0_1;

// ------ Main Program ------
int main()
{
  fdser0_1 = fdserial_open(0, 1, 0b0000, 2400);

}
"

And the code presented to the compiler:

// ------ Libraries and Definitions ------
#include "simpletools.h"

#include "fdserial.h"

#define MY_MYVALUE 128

// ------ Global Variables and Objects ------
fdserial *fdser0_1;

// ------ Main Program ------
int main() {
  fdser0_1 = fdserial_open(0, 1, 0b0000, 2400);

}

It looks like the Blockly core is generating clean code but the C editor is munging it up.

[zfi]

This code in the prettyCode function is removing the CR/LF

  rawCode = jsBeautify(rawCode, {
    'brace_style': 'expand',
    'indent_size': 2,
  });

[PropGit]

There may be a jsBeautify option to preserve trailing CR/LF?

[zfi]

Before call to jsBeautify

"// ------ Libraries and Definitions ------
#include "simpletools.h"
#define MY_MYVALUE  128
#include "fdserial.h"

// ------ Global Variables and Objects ------
fdserial ___REFERENCE_OPERATOR___fdser0_1;

// ------ Main Program ------
int main()
{
  fdser0_1 = fdserial_open(0, 1, 0b0000, 2400);

}
"

After call to jsBeautify:

"// ------ Libraries and Definitions ------
#include "simpletools.h"

#define MY_MYVALUE 128#include "fdserial.h"

// ------ Global Variables and Objects ------
fdserial ___REFERENCE_OPERATOR___fdser0_1;

// ------ Main Program ------
int main()
{
  fdser0_1 = fdserial_open(0, 1, 0b0000, 2400);

}"

Adding an option to preserve newlines is apparently ineffective. (sigh).

[zfi]

It appears that the jsBeautify package is designed to make JavaScript, CSS, and HTML look nice. It does not support formatting C language source code. Disabling the package does correct this issue but it also means that we are no longer overtly trying to make the source code 'pretty'. I am more concerned that the C source code emitted compiles cleanly than making visual adjustments to the code.

Thoughts?

[MatzElectronics]

There was a whole stack of reflex that used to be there to correct what jsbeautify did to break the c code...what happened to it?

On Fri, Apr 24, 2020, 9:53 AM Jim Ewald notifications@github.com wrote:

It appears that the jsBeautify package is designed to make JavaScript, CSS, and HTML look nice. It does not support formatting C language source code. Disabling the package does correct this issue but it also means that we are no longer overtly trying to make the source code 'pretty'. I am more concerned that the C source code emitted compiles cleanly than making visual adjustments to the code.

Thoughts?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/solo/issues/420#issuecomment-619127945, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEWSVOGKZVC7NHDCHYD3WCLROG7W7ANCNFSM4MPVS24A .

[zfi]

I don't know what reflex is but I cannot find any reference to in the the history on the blocklyc.js file. Do you have a version of the file that contains this missing code that we can look at?

[MatzElectronics]

Sorry, autocorrect... regex

On Fri, Apr 24, 2020, 10:03 AM Jim Ewald notifications@github.com wrote:

I don't know what reflex is but I cannot find any reference to in the the history on the blocklyc.js file. Do you have a version of the file that contains this missing code that we can look at?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parallaxinc/solo/issues/420#issuecomment-619133589, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEWSVOHNMCTKWA2GUO4TOCTROHA6LANCNFSM4MPVS24A .

[zfi]

This is what the function looks like now. Previous versions omit the code for the reference and arrow operator bits. Is there code missing from this?

const prettyCode = function(rawCode) {
  // Prevent JS beautify from improperly formatting reference, dereference, and arrow operators
  rawCode = rawCode
      .replace(/\*([_a-zA-Z()])/g, '___REFERENCE_OPERATOR___$1')
      .replace(/([_a-zA-Z()])\*/g, '$1___REFERENCE_OPERATOR___')
      .replace(/&([_a-zA-Z()])/g, '___DEREFERENCE_OPERATOR___$1')
      .replace(/->/g, '___ARROW_OPERATOR___');

  // run the beautifier
  rawCode = jsBeautify(rawCode, {
    'brace_style': 'expand',
    'indent_size': 2,
  });

  // restore the reference, dereference, and arrow operators
  rawCode = rawCode.replace(/,\n[\s\xA0]+/g, ', ')
      .replace(/___REFERENCE_OPERATOR___/g, '*')
      .replace(/___DEREFERENCE_OPERATOR___/g, '&')
      .replace(/___ARROW_OPERATOR___/g, '->')

      // improve the way functions and arrays are rendered
      .replace(/\)\s*[\n\r]\s*{/g, ') {')
      .replace(/\[([0-9]*)\]\s*=\s*{\s*([0-9xXbBA-F,\s]*)\s*};/g,
          function(str, m1, m2) {
            m2 = m2.replace(/\s/g, '').replace(/,/g, ', ');
            return '[' + m1 + '] = {' + m2 + '};';
          });

  return rawCode;
};

[MatzElectronics]

Kind of guessing here, but there is a good chance we modified jsbeautify back in the day before we know not to mess with the libraries themselves. Within jsbeautify there is a this.line_starters variable that contains #include, which I doubt came with the library individually.

My suggestion would be to add a bit of regex to the output and add line-breaks before any #[a-z]+

[zfi]

We're going to remove the beautifier package since the way it is being applied in this project is unsupported. We can address any side issue that may come from this decision as they present themselves.