IDE 1.6.6 breaks sketches that use multiline function declarations.

[ffissore]

From @bperrybap on December 6, 2015 23:43

The latest 1.6.6 IDE breaks sketches that use to and still should compile.
The issue is that the IDE is incorrectly inserting its prototypes when it converts the sketch to a .cpp file.
It appears that it is looking for first line of the first function and then attempting to insert the prototypes on the line just above it.
When the function declaration is not all on a single line, because the return type is on a line above the reset of the declaration, the IDE inserts the sketch function prototypes below the return type of the function declaration which causes the code to no longer compile.
This has broken many of my sketches and has broken some of my openGLCD library example sketches.
While the code in my sketches and library are much more complex and do not involved the setup() or loop() functions, here is a minimal example that demonstrates the problem.

void
setup(){}

void
loop() {}

If you go look at the .cpp file generated, you can see the problem:

#include <Arduino.h>
#line 1
#line 1 "/media/UbuntuRoot/home/bill/Arduino/Arduino-AVR/sketches/test/Test166/Test166.ino"


void
void setup();
void loop();
#line 4
setup() { }

void
loop() {}

Notice that the prototypes were inserted below the return type of the first function.
Curiously, it does manage to locate the and generate the correct prototype even if there are multiple whitespace lines between the return type and the reset of the function declaration or even if the rest of the declaration is on multiple lines like say the function name, parens, or arguments are on different/multiple lines.
The issue seems to be it is inserting the prototypes just above the line that contains the function name rather than just above the line that contains the return type.

Copied from original issue: arduino/Arduino#4265

[ffissore]

Sorry, that's a known limitation, caused by ctags, a tool used by arduino-builder. Until @cmaglie comes up with a clang based replacement, please write your functions on one line only

[bperrybap]

This is a new limitation that has been created that is different from the prior issues, so at a minimum it should be documented as a known issue and included in the release notes with work arounds from now on until it is fixed.

And it isn't that all functions must be declared on a single line.
The only issue is with the first function in the sketch.
And even then it doesn't have to be on a single line, the limitation is that the return type for the very first function declaration has to be on the same line as the function name

i.e.

int foo
( uint8_t bar
)

is just fine.

Also, if a sketch has function declarations above an include that defines types used for other functions declared lower in the module, then the prototypes for those lower functions will end up being inserted prior to the include for the header that defines the types used within the prototype.
i.e.

int foo() // first function in module
{
// do some stuff
}
#include <libheader.h>
foolibtype bar(barlibtype x)
{
// do some stuff
}

In the above case, the prototype for bar() will be inserted above foo() but the problem is it uses types that are not yet known since the libheader.h include will be included below the prototype.

It also appears that if you have a prototype for the very first function in your sketch, that no additional prototypes will be generated. There can still be other prototypes in the module, just not a prototype for the first function.

The problem is that the current behavior creates somewhat unpredictable errors whose cause is invisible to a user that is looking at his original sketch code that used to work in prior IDE versions.
i.e. The IDE now creates errors on code that used to work and should still work.

The only way to know what really happened is to go down and locate the .cpp file which the average Arduino will not know how to do.

So while the real issue can be tagged as a "worn't fix", it needs to at least create a documentation issue that puts this limitation as a known issue in the release notes along with possible work arounds.
LIke ensuring that the first function in the sketch puts its return type on the same line as the function name when declaring the function.

Alternatively, the sketch can declare a dummy function as the fist function to "trick" the auduino-builder to behave properly.
something like:

int dummyfunc(){}

This same kind of goofy stuff was necessary in the older IDEs (but it looked for a variable) to create a proper insertion point for the Arduino.h header. Without that it corrupted your code by missinserting the include for that.
So this is a similar insertion point issue but for the prototypes instead of the arduino header include.

All the rules/limitations should be documented with work arounds so that it doesn't continue to crop up as a reported bug.

[ffissore]

bperrybap ha scritto il 07/12/2015 alle 19:22:

And it isn't that /all/ functions must be declared on a single line.
The only issue is with the /first/ function in the sketch.
And even then it doesn't have to be on a single line, the limitation is
that the return type for the very first function declaration has to be
on the same line as the function name

All functions must be on one line, otherwise prototypes won't be generated

Also, if a sketch has function declarations above an include that
defines types used for other functions declared lower in the module,
then the prototypes for those lower functions will end up being inserted
prior to the include for the header that defines the types used within
the prototype.

That's why usually #include are put at the top of the sketch. If you
like to mix them, add the prototype yourself: builder will see you
already have the prototype and skip its generation

The problem is that the current behavior creates somewhat unpredictable
errors whose cause is invisible to user that is looking at his original
sketch code that used to work in prior IDE versions.
i.e. The IDE now creates errors on code that used to work and should
still work.

The only way to know what really happened is to go down and locate the
.cpp file which the average Arduino will not know how to do.

So while the real issue can be tagged as a "worn't fix", it needs to at
least create a documentation issue that puts this limitation as a known
issue in the release notes along with possible work arounds.
LIke ensuring that the first function in the sketch puts its return type
on the same line as the function name when declaring the function.

/cc @cmaglie

It also appears that if you have prototype for the very first function
in your sketch, that no adidtional prototypes will be generated. There
can be other prototypes, just not a prototype for the first function.

That's because all functions must be on one line

This same kind of goofy stuff was necessary in the older IDEs (but it
looked for a variable) to create a proper insertion point for the
Arduino.h header. Without that it corrupted your code by missinserting
the include for that.

Arduino.h is now included at the top of the file, at its first line

[bperrybap]

All function declarations do not not need to be on one line in order to get function prototypes.
It isn't that simple nor restrictive. I often do not put the return type on the same line as the rest of the function and the prototypes are still created.
And that is why I think restrictions like this need to be documented and/or handled better.
Note: when I say "IDE" below I am not really distinguishing between the actual IDE vs a tool like arduino-builder that might actually be doing the work.

The issue is a regression of the prototype list insertion point.
Maybe the IDE could (should) detect when it can't reliably insert a prototype list and print an error and exit rather than blindly misinserting the prototype list - which creates compiler errors unrelated to the origin source code.

If there is no other better/smarter work around for handling the prototype list insertion point in the IDE/arduino-builder code,
it would be from detecting that the fist token (the return value) of the prototype is not the fist token on the line being used as an insertion point, this will happen whenever the return value is not on the same line as the function name in the function declaration..
(But if the IDE is going to be looking at the return value token and doing checks using it, it could just as well continue to back up line by line from the line with the function name until it found the real line that contained the return value token (ignoring leading whitespace) and use that line instead of the line with function name as the insertion point)

I did some additional testing to verify the actual limitations in 1.6.6

With the current parsing & insertion logic in 1.6.6, the only function that must have its return value on the same line as the function name is the first function of the sketch.
The others are free to do whatever they want.
(Given the previous ctags comment I'm assuming the parsing code uses a function line # -1 of the first function given by ctags to determine the insertion point?)

Here is an example that shows how declarations do not need to be on one line but the first function must at least have its return value and function name on the same line.

void foo
(int x
)
{
    bar(x);
}

void
bar(
int
xval
)
{
    exit(xval);
}

void setup() { }

void
loop()
{
int x = 0;

    foo(x);
}

If you look at the .cpp file generated by IDE 1.6.6, all the prototypes are correct and everything works. Interestingly, 1.6.6. creates a single line prototype while older IDEs will create the prototype the same way the function was declared.
Currently, the only limitation on function declarations in a sketch is that very first function in the sketch must have its return value on the same line as the function name.

My view is that all s/w releases should have a list of "known issues", particularly for a project as large as Arduino.
This is particularly helpful for documenting regressions.
It would be nice for the IDE to have a known issues area where regressions like this or other known issues/problems can be documented to give developers a warning of potential issues.
Maybe they could even even be github issues that get a special category so that they can automatically be extracted when creating the release notes.

In this case it would be helpful because there is situation where code that used to work on the older IDEs no longer builds on 1.6.6 and it takes a bit of poking around to figure out why and how to work around the issue since the issue/error is unrelated to the actual users source code but is a case of the IDE silently misparsing the users source code.

It is actually a bit surprising to me that a project as large as Arduino doesn't have a known issues section in the release notes.

There seems to be hundreds of these types of little "gotchyas" in arduino and those that have been using it and playing with for quite some time are familiar with them. (There are many things that I have had to work around over the years and still continue to work around)
Currently as far as I know there is no place for anybody to document or read about them.

[ffissore]

Thank you @bperrybap for your detailed description.

I'm sure @cmaglie will find a way to communicate these issues/gotchas better than we do now. The whole point of arduino-builder was to remove those gotchas (take a look at the number of closed issues related to preprocessing).

However, it relies on ctags for listing functions and ctags fails to understand at which line a function starts when its return type is on a line of its own

@cmaglie is working on a drop-in replacement of ctags. This will also help solving #68

[lmihalkovic]

@cmaglie is working on a drop-in replacement of ctags. This will also help solving #68

Although I think I have a sense of what he is up to (clang lib I imagine), I hope for the community that there was a good planing of this solution. At the risk of sounding naive, despite plenty of time the simple java-regex-based Compiler.java could not be stabilized enough to address the long list of known issues, and the recent 2 months writing of what was going to be THE solution landed with this and other similarly still not addressed issues (this issue cannot be a surprise considering how long ctags has been around).

In the meantime, a large number of issues are pilling up in the main tracker, some of them for years. This for a small example of mis-appreciation of the seriousness of the situation, and there are plenty of bigger ones. The team has plenty of courage to tackle problems, but is it possible that it might be beneficial to review how issues are evaluated and prioritized?!

[focalintent]

I'm not convinced the problem is ctags. With this code - https://gist.github.com/focalintent/9cb049195962b677b375 - the hourly build from a few hours ago fails to compile and make prototypes. However, here's the end of the output from the ctags command (which I copied out of the ide and ran from the command line):

setup   /Users/dgarcia/Dropbox/Arduino/DiscoStrobe/DiscoStrobe.ino  /^void setup() {$/;"    kind:function   line:1  signature:()    returntype:void
loop    /Users/dgarcia/Dropbox/Arduino/DiscoStrobe/DiscoStrobe.ino  /^void loop()$/;"   kind:function   line:5  signature:()    returntype:void
first   /Users/dgarcia/Dropbox/Arduino/DiscoStrobe/DiscoStrobe.ino  /^void first()$/;"  kind:function   line:11 signature:()    returntype:void
second  /Users/dgarcia/Dropbox/Arduino/DiscoStrobe/DiscoStrobe.ino  /^void second($/;"  kind:function   line:20 signature:( uint8_t dashperiod, uint8_t dashwidth, int8_t dashmotionspeed, uint8_t stroberepeats, uint8_t huedelta) returntype:void
third   /Users/dgarcia/Dropbox/Arduino/DiscoStrobe/DiscoStrobe.ino  /^static void third($/;"    kind:function   line:29 signature:( uint8_t startpos, uint16_t lastpos, uint8_t period, uint8_t width, uint8_t huestart, uint8_t huedelta, uint8_t saturation, uint8_t value)   returntype:void

ctags is correctly pulling out the name, kind, signature, and return type for all the functions in the ino file. However, the arduino ide is failing to create prototypes for functions second and third.

edited

Ran arduino-builder with debug output - it's because the regex for the function and the given prototype aren't matching up (or the code responsible for deciding whether or not to skip a tag is too restrictive):

===debug ||| Skipping tag {0}. Reason: {1} ||| [second arduino.cc%2Fbuilder%2Fctags.prototypeAndCodeDontMatch]
===debug ||| Skipping tag {0}. Reason: {1} ||| [third arduino.cc%2Fbuilder%2Fctags.prototypeAndCodeDontMatch]

ctags is capable of parsing out all the parameters to the function, as evidenced by the signature coming out of ctags being correct.

[focalintent]

Pull request #117 is an attempt to fix this particular issue (unless @cmaglie's drop in ctags replacement will flatten multi-line prototypes into the /^...$/ block, the problem in this issue will still exist)

[stefandz]

I can report that #117 fixes this on my Arduino 1.6.7 setup - I tested binaries provided by @facchinm.

[per1234]

@facchinm can this be closed by fbfaa56?

[bperrybap]

@per1234, is that update in 1.6.8? I just tried 1.6.8 with the example below and it still fails:
If that code update is in 1.6.8 and was supposed to fix this issue, it doesn't.
It doesn't work on 1.6.7 either.

void foo (int x)
{
    bar(x);
}

void
bar ( int xval)
{
    exit(xval);
}

void setup() { }

void loop()
{
int x = 0;
    foo(x);
}

[per1234]

@bperrybap no it's in the hourly build but that code still doesn't compile. I had only tested your first example:

void
setup(){}

void
loop() {}

Which does compile now but I notice it started working in 1.6.7 so that's unrelated to fbfaa56, which I'm guessing is what fixed arduino/Arduino#4843. So I was wrong and this issue was not fixed by fbfaa56. Sorry about that. Per

[facchinm]

@bperrybap and @per1234 , I just checked with the provided sketch and indeed the issue is not solved in nightly. ctags reports the line of the function name, not the return type, so all the code involved in finding the complete declaration starts too late.
I'm trying to solve this asap, thanks for reporting

[facchinm]

Will be fixed by merging #182