Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IDE doesn't honor preprocessor conditionals when including libraries #1841

Closed
ghost opened this issue Feb 2, 2014 · 27 comments
Closed

IDE doesn't honor preprocessor conditionals when including libraries #1841

ghost opened this issue Feb 2, 2014 · 27 comments
Assignees
Labels
Component: IDE The Arduino IDE Component: Preprocessor The Arduino sketch preprocessor converts .ino files into C++ code before compilation
Milestone

Comments

@ghost
Copy link

ghost commented Feb 2, 2014

In the Arduino IDE 1.5.5, the default sketch on the Uno compiles to 466 bytes:

//this compiles to 466 bytes
void setup() {
}

void loop() {
}

Including Servo.h, it compiles to 1104:

//this compiles to 1104 bytes
#include <Servo.h>

void setup() {
}

void loop() {
}

I would therefore expect the following code to compile to 466 bytes, but this is not the case:

//this compiles to 1104 bytes even though Servo.h should not be included
#ifdef SOME_MACRO_THAT_IS_NOT_DEFINED
#include <Servo.h>
#endif

void setup() {
}

void loop() {
}

However, commenting out the include statements does prevent the library from being included:

//this compiles to 466 bytes as expected
//#ifdef SOME_MACRO_THAT_IS_NOT_DEFINED
//#include <Servo.h>
//#endif

void setup() {
}

void loop() {
}

This caused some headache for me as I tried to figure out why my sketch size was so large despite not including any libraries. This is not the behavior I think anyone would expect from the IDE.

@matthijskooijman
Copy link
Collaborator

What happens here is that the Arduino IDE finds #include lines in the source and, based on those, includes libraries in the link. In your example, when actually compiling your sketch, Servo.h will not be included and no functions inside it will be called, but the Servo library will still be included in the link.

AFAIU, any functions inside the Servo library that are not used should be removed from the final link (because of --gc-sections). However, if things like ISRs are involved, things might become more complicated (and there are probably other reasons for including a function even though it's not strictly needed).

As for solving this - that might prove difficult. Naïvely, I would say to just run gcc in preprocessor-only mode (gcc -E) and then look for #include directives in the preprocessed source. However, #include itself is also processed by the preprocessor, so the preprocessor would throw an error (since we don't know what libraries should be included in the include path yet).

We could again solve this by doing a two-step process: First apply the current naïve "find #include directives" filter, which results in a list of potentially included libraries. Add those to the include path and run gcc -E to generate the final list of include paths (adding -dI would additionally keep #include directives in the preprocessed output, so we can look for those). This is tricky to get right, though, since we'd also have to apply the first step recursively to (potentially) included libraries to make sure all #includes in the second step can be resolved.

An alternative would be to use gcc -M, which generates a Makefile snippet listing all the dependencies of a file (e.g., headers it includes). Normally, it only lists the header files it can actually find in the include path (or current directory), but when you add -MG, it will also list files that it could not find (using the name used in the #include directive instead of the full path). Ideally you'd want to use -MM instead of -M (the former suppresses output of system headers), but that also causes -MG to work for #include "foo.h" only and not for#include <foo.h>`, which makes it useless.

To really make this work reliably (e.g., to allow the sketch to only #include a library depending on a #define in another library), this should be applied recursively. With recursively, I mean to run gcc -M -MG on the sketch as long as any header files are still unresolved. For every unresolved header, find the the library that supplies it, add it to the include path and then run gcc -M -MG again. The upside here is that you have a single way to recursively detect the dependencies of a sketch and its dependent libraries at the same time (fixing #236). The downside again is that in the final list of dependencies, you don't know which dependencies belong to the sketch and which to libraries, which might pose limits on user feedback in feature GUI improvements.

@ffissore ffissore added the IDE label Feb 27, 2014
@euphy
Copy link

euphy commented Oct 5, 2014

This upsets me a lot. Unless I'm wrong, it makes conditional #includes impossible. I can't figure out what the recommended work around would be if I want to conditionally include either one OR another library that have the same interfaces.

The example I am working with is Adafruit's motorshields:

#ifdef ADAFRUIT_MOTORSHIELD_V1
#include <AFMotor.h>
#endif

#ifdef ADAFRUIT_MOTORSHIELD_V2
#include <Adafruit_MotorShield.h>
#endif

Which results in

Adafruit_MotorShield\Adafruit_MotorShield.cpp.o:(.data.microstepcurve+0x0): multiple definition of `microstepcurve' 
AFMotor\AFMotor.cpp.o:(.data.microstepcurve+0x0): first defined here

Not a C expert of course, but the docs I read about it seem to suggest this is a kosher preprocessor pattern.

Accept that this is a hard problem to fix - but it can't be uncommon in exactly this kind of situation.
Is there a workaround?

@barbudor
Copy link

barbudor commented Oct 5, 2014

Dear Sandy,

This is a known behavior of the Arduino IDE.
In order to get working conditionnal #include you need to place them into a
separated H file like :

*Your_sketch.ino *
#include "conditional_includes.h"

....

conditionnal_includes.h

#ifdef ADAFRUIT_MOTORSHIELD_V1
#include <AFMotor.h>
#endif

#ifdef ADAFRUIT_MOTORSHIELD_V2
#include <Adafruit_MotorShield.h>
#endif

This should work

2014-10-05 16:40 GMT+02:00 Sandy Noble notifications@github.com:

This upsets me a lot. Unless I'm wrong, it makes conditional #includes
impossible. I can't figure out what the recommended work around would be if
I want to conditionally include either one OR another library that have the
same interfaces.

The example I am working with is Adafruit's motorshields:

#ifdef ADAFRUIT_MOTORSHIELD_V1
#include <AFMotor.h>
#endif

#ifdef ADAFRUIT_MOTORSHIELD_V2
#include <Adafruit_MotorShield.h>
#endif

Which results in

Adafruit_MotorShield\Adafruit_MotorShield.cpp.o:(.data.microstepcurve+0x0): multiple definition of `microstepcurve'
AFMotor\AFMotor.cpp.o:(.data.microstepcurve+0x0): first defined here

Not a C expert of course, but the docs I read about it seem to suggest
this is a kosher preprocessor pattern.

Accept that this is a hard problem to fix - but it can't be uncommon in
exactly this kind of situation.
Is there a workaround?


Reply to this email directly or view it on GitHub
#1841 (comment).

@euphy
Copy link

euphy commented Oct 5, 2014

Thanks @barbudor that makes some sense - and indeed it does correctly prevent the includes from happening if neither case is true. I suppose it is obvious now that I need to be able to specify the exact path to the libraries in the conditional_includes.h because the IDE won't do it's auto-find magic on the #includes any more. The include fails with a "no such file or directory" without further intervention.

I see the problem! Thanks for guide.

@nickgammon
Copy link

The IDE does not honor conditional preprocessor conditionals when generating function prototypes either. For example:

void setup() 
{ 
}
void loop() 
{ 
}

#if 0
foo bar ()
{
}
#endif

This gives the error:

sketch_oct10d:4: error: ‘foo’ does not name a type

Also, the line number (4) is wrong. I understand why this is happening, but it is frustrating trying to explain to forum users who post stuff like "but it should be ignoring foo!".

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

Much of these problems could be resolved with a project.properties and library.properties files loaded in before a compile. As far as the ignoring stuff, yes, that is IMHO a total bug.
The way to solve this particular issue is to pass the code thru the pre-processor first, and THEN look for include statements.

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

Here is an example on how to pick out the needed includes from command line on Linux:

gcc -x c++ -E sketch.ino 2>&1 | grep ": error: " | cut -f5 -d':'

Certainly something this simple can be done. I could patch it myself with ease.

I could even add the extra properties stuff too. As a shield driver developer I find it very frustrating to tell users that they need to modify headers in a library every time they switch to a project that has a different setup. Its ridiculous, considering these fixes are so easy to do. I really don't understand the reasoning behind rejecting modifications that would help everybody, especially someone who is new to everything, and they get all nervous when they have to edit something unrelated to their project, and/or forget to make changes and end up blaming the dev.

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

Even better:
gcc -x c++ -E xmemdemo.ino 2>&1 | grep ": No such file or directory" | cut -f5 -d':'
So that other errors won't interfere.

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

Last time, really. this one is the best:

# echo `gcc -x c++ -E xmemdemo.ino 2>&1 | grep '.h: No such file or directory' | cut -f5 -d':'`
xmem.h

@matthijskooijman
Copy link
Collaborator

There is a fundamental problem here: What include paths do you pass to the preprocessor run to determine the libraries to include? What if some libraries are included conditionally based on defines in another library? There's probably just no perfect DWIM solution here, I'm afraid...

@nickgammon
Copy link

I've posted a "solution" here: http://www.gammon.com.au/forum/?id=12625

Basically you leave the .ino file empty, or at the most, just put the #include statements in for any libraries that you need. That way, it effectively becomes a "project" file simply listing what libraries need to be copied to the compilation area.

Then your main code goes into .cpp (or .c) files which you add to the project, and which are not subject to this pre-processor mangling.

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

The errors from gcc -E tell you what headers to look for to include the libraries.

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

So that you 'get the full idea'...

xmemdemo# for j in `for i in \`gcc -x c++ -E xmemdemo.ino 2>&1 | grep '.h: No such file or directory' | cut -f5 -d':'\` ; do find ../libraries/ -maxdepth 2 -name $i -print ; done` ; do dirname $j; done
../libraries/xmem

Understand now?

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

My point is this...
The java code written to do the pulling out of the wanted include statements basically reinvented the wheel, poorly.
This is a sad attempt, as the compiler pre-processor already knows how to do this kind of thing for you, and is the correct way to determine what bits are missing. Who ever wrote the IDE either missed the ability of the pre-processor to do this for you, or should have asked someone who has experience.

@matthijskooijman
Copy link
Collaborator

I understood your point, and I guess looking at gcc error messages is a decent way to detect library includes. As you say - no point to write another compiler, you'll only do it poorly.

However, I made a more general point - I'll add a short example:

#include <LibraryA.h>
#ifdef LIBRARY_A_NEEDS_B
#include <LibraryB.h>
#endif

Scanning preprocessor output for missing includes shows only LibraryA, while it might also need LibraryB.

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

That one is pretty simple. Here, some pseudo code.

do {
        errors = preprocess_ino(*headers);
        addnew = errors;
        i = 0;
        while(addnew--) {
                addpath(headers[i++]);
        }

} while(errors); 

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

and before you ask... the missing case...

do {
        errors = preprocess_ino(*headers);
        corrected = 0;
        i = 0;
        while(errors--) {
                corrected += addpath(headers[i++]);
        }

} while(corrected); 

@xxxajk
Copy link
Contributor

xxxajk commented Nov 6, 2014

I love sh so much... I just had to do it...

q='' ; while true; do k=$(for j in `for i in \`gcc -x c++ -E $q xmemdemo.ino 2>&1 | grep '.h: No such file or directory' | cut -f5 -d':'\` ; do find ../libraries/ -maxdepth 2 -name $i -print ; done` ; do dirname $j ; done ; ); [ "$k" ] || break; q=$(echo $q ; for i in $k ; do echo "-I"$i ; done ; ); done ; echo $q

LOL

@matthijskooijman
Copy link
Collaborator

The solution you propose crossed my mind and solves the one example I gave, but it does not solve all cases. In general, it seems to be hard to find a way to solve all corner cases. Here's another example (I'm not inviting you to solve it - I'm just trying to show you that this is a non-trivial, perhaps even unsolvable, problem):

#include <LibraryA.h>
#ifndef LIBRARY_A_BREAKS_B
#include <LibraryB.h>
#endif

When A defines LIBRARY_A_BREAKS_B, this will link both library A and B, even though only A was intended.

@xxxajk
Copy link
Contributor

xxxajk commented Nov 7, 2014

The recursion would resolve it properly.
On Nov 7, 2014 2:39 AM, "Matthijs Kooijman" notifications@github.com
wrote:

The solution you propose crossed my mind and solves the one example I
gave, but it does not solve all cases. In general, it seems to be hard to
find a way to solve all corner cases. Here's another example (I'm not
inviting you to solve it - I'm just trying to show you that this is a
non-trivial, perhaps even unsolvable, problem):

#include <LibraryA.h>
#ifndef LIBRARY_A_BREAKS_B
#include <LibraryB.h>
#endif

When A defines LIBRARY_A_BREAKS_B, this will link both library A and B,
even though only A was intended.


Reply to this email directly or view it on GitHub
#1841 (comment).

@xxxajk
Copy link
Contributor

xxxajk commented Nov 7, 2014

Hint... add one new path at a timem....
On Nov 7, 2014 2:39 AM, "Matthijs Kooijman" notifications@github.com
wrote:

The solution you propose crossed my mind and solves the one example I
gave, but it does not solve all cases. In general, it seems to be hard to
find a way to solve all corner cases. Here's another example (I'm not
inviting you to solve it - I'm just trying to show you that this is a
non-trivial, perhaps even unsolvable, problem):

#include <LibraryA.h>
#ifndef LIBRARY_A_BREAKS_B
#include <LibraryB.h>
#endif

When A defines LIBRARY_A_BREAKS_B, this will link both library A and B,
even though only A was intended.


Reply to this email directly or view it on GitHub
#1841 (comment).

@matthijskooijman
Copy link
Collaborator

I guess you mean iteration, not recursion (looking at your C version, not the sh version)? In any case, your proposed code would not handle this case properly. If modified to only add one path per gcc run, it would, but I'm pretty sure that I can keep coming up with counter-examples for this - it's just a complex problem.

One approach that might work, is to do the #include matching as now, so find all #include lines, ignoring any preprocessor directives. Then, add all of the detected libraries to the include path, run the preprocessor and ask which files were actually #included. Then, for the final compilation, only use those libraries.

Tricky part here is how to run just the preprocessor - platform.txt doesn't currently have any lines for this...

@xxxajk
Copy link
Contributor

xxxajk commented Nov 7, 2014

It will be tested in my makefile system in a few days. I'll let you know if
it pans out
On Nov 7, 2014 3:52 AM, "Matthijs Kooijman" notifications@github.com
wrote:

I guess you mean iteration, not recursion (looking at your C version, not
the sh version)? In any case, your proposed code would not handle this case
properly. If modified to only add one path per gcc run, it would, but I'm
pretty sure that I can keep coming up with counter-examples for this - it's
just a complex problem.

One approach that might work, is to do the #include matching as now, so
find all #include lines, ignoring any preprocessor directives. Then, add
all of the detected libraries to the include path, run the preprocessor and
ask which files were actually #included. Then, for the final compilation,
only use those libraries.

Tricky part here is how to run just the preprocessor - platform.txt
doesn't currently have any lines for this...


Reply to this email directly or view it on GitHub
#1841 (comment).

@ffissore ffissore self-assigned this Jan 20, 2015
@ffissore ffissore added the Component: Preprocessor The Arduino sketch preprocessor converts .ino files into C++ code before compilation label Jan 20, 2015
@ffissore
Copy link
Contributor

Please checkout this thread [1] on devs mailing list and try one of the linked IDEs. It should fix the problems reported in this issue
[1] https://groups.google.com/a/arduino.cc/forum/#!topic/developers/4X2T3rCxXWM

@ffissore
Copy link
Contributor

A new version of the new preprocessor is available. Links available at the bottom of this email
https://groups.google.com/a/arduino.cc/forum/#!msg/developers/4X2T3rCxXWM/YNJl6PZuDucJ

@ffissore
Copy link
Contributor

New preprocessor tracked at #2636. Builds for testing it are available

@ffissore
Copy link
Contributor

Fixed by #3779

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: IDE The Arduino IDE Component: Preprocessor The Arduino sketch preprocessor converts .ino files into C++ code before compilation
Projects
None yet
Development

No branches or pull requests

6 participants