Fix logic to retrieve correct line insertion

[facchinm]

Fixes #199

[facchinm]

@alto777 @bperrybap could you test with the release produced by @ArduinoBot ?

[ArduinoBot]

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/arduino-builder/arduino-builder-200.zip

ℹ️ To test this build:

1. Replace arduino-builder binary (you can find it where you installed the IDE) with the provided one

[alto777]

I believe I have carefully tested as you request. The OS X arduino-builder placed into the 1.6.13 IDE:

The blanks lines no longer cause the problem!

[bperrybap]

The linux32 bit version appears to have resolved the issue on Mint 17.1 in IDE version 1.6.13 IDE.
@facchinm this is great news.
It looks like it was some sort of confusion of line number mistake between files but could you post a brief description here of what the issue was and how it was fixed just for historical purposes (and my own and probably as well as others curiosity)

[facchinm]

Sure!
In fact, the preprocessor needs to know where it can insert the generated prototypes. The strategy is:

• not too early (some stuff could be declared by user includes)
• not too late (need to be placed before first use of any function which prototype is being generated)

To do so, it scans the tags for the line they are declared in and searches for the smallest of these numbers. Then it searches again, in case a function has been used as a function pointer (thus its prototype need to be declared before this usage).

The bug was due to the fact that the search is performed on all sketch files, and the prototypes inserted only in the "main" one.

If the main file had less lines than the minimum prototype line number, if was falling back using a safe place, so the bug was invisible.
Instead, the problematic sketch had this empty but long main file, with multiline comments in it, so the prototypes could be pasted anywhere (also in the comment section) quite "randomly".

Adding a function to the main file "solves" the issue since it finds a safe hook.
Only scanning for line tags on main file solves the issue in the proper way 😉

[bperrybap]

Speaking of that "not too early", I have seen some issues in some of my stuff where prototypes are generated and the functions have an argument that uses a type that is declared in a library header file but the prototypes are inserted before the include for the header file that declares those types.
Are you saying the builder should be smart enough to insert prototypes after all the includes?
If so, I need to file an other builder issue for this if this is not resolved in this latest builder.

[matthijskooijman]

@facchinm, wouldn't it be a better solution to actually insert the prototypes into a secondary file if needed? Or at least at the end of the primary file (IIUC the current code puts them at the top of the primary file now if there are no functions in the primary file?).

I recall that the code that handles prototype insertion might also need some refactoring to improve other things, but I can't recall off-hand what those improvements were or where they were discussed.

[matthijskooijman]

@bperrybap, no arduino-builder doesn't actually detect all dependencies between lines of code, it just uses some heuristic (insert just before the first function declaration / definition), combined with very simple dependency checking (to detect a global variable that references a function pointer). In your case, it sounds like it should be a matter of moving the #include for that library upwards, since apparently there is something before it that triggers the protoype insertion already.

[bperrybap]

@matthijskooijman
I'm not expecting anything to detect dependencies between lines of code - not sure what purpose that would solve.
In the case I was referring to, there was a single sketch file and the needed includes were above all functions and all global declarations.
I can't remember if it was a misinsertion issue or whether the builder bombed out because it does a pass on the sketch file without the library include paths set yet. BTW, I've run into other issues related to this. In this case, even if I declared my own prototype, the code still could not be built.
I re-wrote the code to avoid the dependent type.
Sounds like I need to re-create it and post another issue.

[matthijskooijman]

I'm not expecting anything to detect dependencies between lines of code - not sure what purpose that would solve.

In your case, the function declaration depends on the include, because it needs the type declarations inside the included header file. To perfectly insert prototypes, these kind of dependencies would need to be known (which isn't possible with the current approach).

Sounds like I need to re-create it and post another issue.

Might be useful, yes. It could very well be "intended" (in the sense that it is known limitation of the approach used), but there's also a chance it's a corner case that should be fixed.

[bperrybap]

anticipated is would probably a better choice of word than intended. I'll continue this discussion of it elsewhere rather than here since it is a distinctly different issue.

[bperrybap]

Just one last followup even though this is off topic for this issue since I didn't want to leave this hanging for this fix/merge.
After going back and re-creating it, In my case, the issue has been resolved in the newer IDEs since the builder is smart enough to insert the generated prototypes just before the first function it sees that ensures that all includes needed are included. VERY NICE! The issue in my case is being an example in a 3rd party library that is trying to ensure that the code can also run in older IDEs (like clear back to 1.0) which are not that smart. So the code breaks on older IDEs and there is obviously no solution for that other than to write the code differently or require a newer version of the IDE.
The specific situation was a function that was returning an enum that was part of a class.
The work around was to use defines instead - not ideal since the defines pollute the namespace which runs the risk of name collision with other libraries.
This specific issue was resolved in IDE 1.6.6 when builder was introduced but there are so many other code land mines that have to be dodged in IDEs before 1.6.8 and also many great new end user ease of use capabilities for 3rd party libraries since IDE 1.6.2 that I may opt to just require IDE 1.6.8 or newer to make things much simpler for the end user and the code maintenance.

Sorry for the distraction.