Skip to content

#include or #error inside #if not working as expected #33

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

Closed
NicoHood opened this issue Oct 9, 2015 · 21 comments
Closed

#include or #error inside #if not working as expected #33

NicoHood opened this issue Oct 9, 2015 · 21 comments
Milestone

Comments

@NicoHood
Copy link

NicoHood commented Oct 9, 2015

Ref: #18
Used official Leonardo board

New issue, just try this sketch:

#include <Arduino.h>
#if !defined(USBCON)
#error
#endif

void setup() {
  // put your setup code here, to run once:

}

void loop() {
  // put your main code here, to run repeatedly:

}

I have the same error within libraries unless I include Arduino.h. Not sure if this is intended, not sure if Arduino.h Includes the register definitions but I think this should be passed by default since things like DDRB should always be accessible without arduino layer.

However its weird that you can inlcude Arduino.h and it works then. And eve more weird that this does NOT help in a simple sketch.

@matthijskooijman
Copy link
Collaborator

I have the same error within libraries unless I include Arduino.h. Not sure if this is intended, not sure if Arduino.h Includes the register definitions but I think this should be passed by default since things like DDRB should always be accessible without arduino layer.

Inside libraries, no preprocessing is done, so you will always need to include what you need. Including Arduino.h is easiest, since that will get you mostly everything. This has been the case as long as I remember.

However, inside the sketch it should just work. I just tried your sketch, and got the same error. Something weird is going on here. Look at the command used to compile the sketch:

"/home/matthijs/docs/Electronics/Arduino/Arduino-1.5/build/linux/work/hardware/tools/avr/bin/avr-g++" -c -g -Os -w -std=gnu++11 -fno-exceptions -ffunction-sections -fdata-sections -fno-threadsafe-statics  -w -x c++ -M -MG -MP -mmcu=atmega32u4 -DF_CPU=16000000L -DARDUINO=10606 -DARDUINO_AVR_LEONARDO -DARDUINO_ARCH_AVR  -DUSB_VID=0x2341 -DUSB_PID=0x8036 '-DUSB_MANUFACTURER="Unknown"' '-DUSB_PRODUCT="Arduino Leonardo"'  "/tmp/buildb2ade5cdfb0cba832a5548a0d4d12d00.tmp/sketch/sketch_oct13a.ino.cpp"

Note that this does not include any -I options, so I don't think Arduino.h will be found. This might not trigger an error, but just a warning, which seem to be disabled by the -w option (which is weird, since the preferences have warnings enabled - I'll report a separate bug about this).

If I remove the #error, compilation works again. Looking at the commands, I see that g++ is called a few times on the sketch, each time adding some include paths. I suspect that this means that this is not actually compiling the sketch, but just doing preprocessing to collect missing includes, and this process doesn't currently support handling preprocessor errors. I suspect that these errors should just be ignored, if there are any missing includes before the error.

However, it also seems that these initial preprocessing attempts are done without Arduino.h added (looking at the preprocessed .cpp file), which means that if libraries are being selected based on stuff from Arduino.h, the preprocessing will not properly match the final compilation.

@NicoHood
Copy link
Author

I also had the "error" sometimes, that a warning appeared and after another warning was solves the other warning disappeared. Even though it was clearly not there. Might be so minor that it ignores it then?

@ffissore ffissore modified the milestones: 1.0.1, 1.0.2 Oct 27, 2015
@ffissore
Copy link
Contributor

Indeed that first gcc call is gcc -M which is used to gather includes. However, gcc does execute directives, thus the error triggered by #error. Unless you have suggestions, I would close this as wontfix, as I don't know a better way to do arduino magic

@matthijskooijman
Copy link
Collaborator

I guess that adding the #include <Arduino.h> line _before running the gcc -M command would help in this case. Additionally, adding library include paths one by one (as originally suggested by @PaulStoffregen) would help to prevent similar problems for defines from library header files (as well as preventing issues with "oscillations" in the library include list / issues with libraries being included when they should not be).

@NicoHood
Copy link
Author

I need to include it in libraries as well which is a bug I think. I dont think that I need to include Arduino.h for register names. there must be something wrong here? The sketch above was just a simpler showcase, but the real (important) bug is inside libraries.

@matthijskooijman
Copy link
Collaborator

@NicoHood, I'm not sure I understand your last comment? You're saying you must add #include <Arduino.h> inside your library source files? That is totally expected and has been like that always. Or did I misunderstand?

@NicoHood
Copy link
Author

Yes. Do I need to include Arduino.h just to get the USBCON definition? I remombered (might be wrong) To not need this, since its a normal reg and those register definitions should be passed via compiler?

@cmaglie
Copy link
Member

cmaglie commented Oct 28, 2015

Register definitions for AVR are contained in avr/avr/include/avr/iom* files of the avr-gcc compiler. In the iom32u4.h I found this:

#define USBCON _SFR_MEM8(0xD8)
#define VBUSTE 0
#define OTGPADE 4
#define FRZCLK 5
#define USBE 7

This file is automatically included by the compiler (I think when you specify the CPU with the -mmcu=... paremeter).

@matthijskooijman
Copy link
Collaborator

@NicoHood You need to include avr/io.h to get USBCON (which is included by Arduino.h, so that also works).

@cmaglie - iom_.h are *not_ automatically included - -mmcu sets the proper define, which is used by io.h to figure out which of the iom*.h files to include.

@cmaglie
Copy link
Member

cmaglie commented Oct 28, 2015

Right, thanks @matthijskooijman !

@ffissore
Copy link
Contributor

Thanks everybody. I guess the solution is to add platform includes (-I) to the command line of gcc -M and gcc -E

@ffissore
Copy link
Contributor

This way it will be possible to get USBCON by including Arduino.h

@ffissore ffissore added bug and removed question labels Oct 28, 2015
@NicoHood
Copy link
Author

NicoHood commented Nov 6, 2015

I now found the real cause of this problem I think.

I recently remove the Arduino include in my project and though it was fixed:
NicoHood/HID@e46469d#diff-c6422d0168a30477f605837b1eda11d9L30

It was until I recompiled my own examples. What happens is:

  • The library files automatically see USBCON definitions (include avr/io.h or something like that).
  • The .ino sketch does NOT see this definitions (which should be fixed)
  • The problem does not occur in my personal sketch, because I include a library before, that uses Arduino.h (aka avr/io.h)

This means we have a serious problem here with the preprocessor. To get away from the USB stuff imagine something like that. The error appears, but the code (with commented error) compiles fine. Even including Arduino.h does not help. There is something wrong, please reopen this issue. Otherwise I cannot use my library (and other libraries as well) correct.

#if !defined(PIN)
#error
#endif

void setup() {
  int t = PINB;
}
void loop() {}

@matthijskooijman
Copy link
Collaborator

I recently remove the Arduino include in my project and though it was fixed:
NicoHood/HID@e46469d#diff-c6422d0168a30477f605837b1eda11d9L30

I don't think this is the right way to approach this. If your library needs Arduino.h, it should include it. It should not rely on being included from the .ino file or some other place that has previously included Arduino.h, since that is fragile, but also will not work for the library's own .cpp files (which will only include your own .h files, with the .ino file not being invovled). This might result in Arduino.h being included twice, but the include guards make sure it only takes effect the first time.

In any case, the real issue here isn't fixed, so I'm reopening this issue. @ffissore now added the include path for Arduino.h so libraries can explicitly include it, but this doesn't work for the sketch. During normal compilation, it gets Arduino.h inserted automatically, but not during dependency-scanning, which means the depenency-scanning works in a different environment. When dependencies are selected based on defines from Arduino.h, this means that the results will be incorrect, but when #errors are given based on defines from Arduino.h, this means that compilation will needlessly fail (an example of the latter is the !defined(PIN) sketch by @NicoHood in the previous comment).

This issue can occur more generically, when dependency selection, or #error directives happen based on defines from an earlier include. Since initially none of the includes in a library actually work, the dependency-scanning preprocessor run will behave as if all includes define no symbols whatsoever, which might results in the wrong includes being selected, or an inappropriate error.

As an example of the latter case, consider a library containing:

#include <SPI.h>                                                               

#if !defined(SPI_HAS_TRANSACTION) || !SPI_HAS_TRANSACTION                      
#error "This library requires Arduino IDE 1.5.8 or above, supporting the SPI transaction API"
#endif 

This is from a real-world library that is now failing, but just putting these lines a libraries/Test/Test.h file and including that file from a sketch is sufficient to trigger this problem. With the above lines, the error is triggered, even though SPI.h defines SPI_HAS_TRANSACTION. However, during the dependency-scanning preprocessor run, SPI.h is not yet on the include path, so the error triggers and compilation halts.

To show incorrect dependencies being selected, a slightly more complex example is needed. Consider two libraries, "Test" and "Foo":

Test.h:

#define HAS_FOO
void foo();

// Other functions here

Test.cpp:

void foo() { }

Foo.h:

void foo();

Foo.cpp:

void foo() { }

It is a bit of a contrived example, but the idea is that "Test" is a
library that added a foo() function in some recent version and uses a
HAS_FOO define to indicate that foo() is available. In earlier
versions of the "Test" library, the foo() function was not included,
but it was available in a separate "Foo" library, that provides just
that function.

A sketch (or other library) wishing to support both older a newer
versions, might contain this:

#include <Test.h>
#if !defined(HAS_FOO)
#include <Foo.h>
#endif

Now, if you compile this, you get:

.../libraries/Foo/Foo.cpp:1: multiple definition of `foo()'
.../libraries/Test/Test.cpp.o:.../libraries/Test/Test.cpp:1: first defined here

What happens is that dependency scanning happens without Test.h in the
include path, so HAS_FOO is not defined, so Foo.h is included. The
dependency scan then adds both libraries to the link, even though Foo
isn't really needed (and in this case actually conflicts with Test).

To fix the errors during compilation, it would be sufficient to simply
ignore non-include errors during the preprocessing step, and only error
out the compilation if the error occurs during the actual compilation.
However, this would not fix the incorrect dependencies example I gave
above.

To fix both of these problems at the same time, the dependency-scanning
should only process the first error of every preprocessing run. If
this is an include error, it should add the corresponding library and
run again. If it is another error, it should bail out. Doing this
ensures that the missing include being processed is really one that is
needed, since no other preprocessor directives can later change it.
This might mean that the number of preprocessor calls becomes a bit
higher, and the limit should probably be raised (also, "depth" is no
longer an appropriate name).

edit: In addittion, the first dependency-scanning preprocessing run should already have the Arduino.h include added for sketches.

Note that this one-by-one approach is exactly the approach proposed by
@PaulStoffregen a long time ago, and I still believe it is the only way
to handle dependency-scanning reliably.

Note that (AFAICS) the preprocessor-based approach also allows arduino-builder to bail out during
the dependency-scanning when an include is present for which no library
is found. With the old regex-based scanning, this was impossible, since
the include might refer to a system file that the scanner didn't know
about. Now that we're using the preprocessing to collect missing
includes, that will handle system files already, so any missing includes
during this step will certainly also result in an error during the
actual compilation. Letting arduino-builder bail out early might allow
giving a clearer error message, such as "None of the installed libraries
offer a Foo.h file" or something like that.

@matthijskooijman matthijskooijman changed the title USBCON not defined (again) #include or #error inside #if not working as expected Nov 9, 2015
@matthijskooijman
Copy link
Collaborator

Note that -Wfatal-errors might be useful to have gcc only print 1 error and then bail out, saving execution time on other errors that aren't going to be processed anyway.

@NicoHood
Copy link
Author

NicoHood commented Nov 9, 2015

My project does not required Arduino.h. But the USBCON available check requires the definitions which are pulled in by arduino.h. And the compile error does not occur because my library cannot see USBCON, it occurs because the sketch cant see it. So normally I do not need to include Arduino.h from what I checked. I just want to clarify this. However its relevant and caused by the bug you mentioned, thats all fine.

@matthijskooijman
Copy link
Collaborator

@NicoHood, I'll not respond to your last comment, since I think the details of how your library should ideally organize its includes would distract from the actual bug here (ping me on IRC if you want to know anyway). As you say, the actual bug you're seeing is that Arduino.h / USBCON is not available in the sketch during preprocessing, which is also illustrated by the !defined(PIN) example you gave earlier, both of which should be fixed with my proposal (note that I made a small edit in my proposal about starting out with Arduino.h included).

@ffissore
Copy link
Contributor

@matthijskooijman I understand what's causing headaches here but unfortunately "preprocessing" = "gcc -M" and "gcc -E". I need a way to prevent gcc from aborting when preprocessing fails. I've found no "ignore-errors" flag, so the only that comes to mind right now is search&replacing #error with #noerror
Any other better idea?

@ffissore
Copy link
Contributor

Actually, #noerror is illegal, so it will be something like //removed error #error...

@matthijskooijman
Copy link
Collaborator

Ah, there is a mismatch between what I thought happened and what actually happens. I have been assuming that you were running the preprocessor and scanning the output for missing include file error messages. I was also thinking that somehow an #error would take precedence, causing compilation to fail, while include errors would not. But looking more closely, it seems arduino-builder lets gcc do preprocessing and generate a dependency file, and scans that for missing include filenames. If an #error happens during this process, preprocessing fails and no output is generated.

So, my proposal doesn't apply cleanly to this approach. You're saying you need a ignore errors flag, but even if you would have that, I'm not sure if the dependency-file based approach allows you to figure out which of the missing includes was first in the source file, which I still believe is essential to make this detection foolproof.

So, how about switching to parsing gcc's output for include errors then? Or was this tried already?

Looking over the gcc manpage for alternatives, I found the -E -dI option, which outputs the preprocessed source with #include directives left in the output. If compilation fails due to a missing include, the last line of the preprocessed output will be the "broken" #include line, which you could simply parse. I think -dI is meant as a debug option, so not sure if it works reliably across gcc versions, but a quick look suggests that his could be made to work. Note that the linemarkers in the output allow distinguishing between a failed include, and a non-existing include. Consider this -dI output:

# 1 "/tmp/buildfc9ac49b4389b81833fcbab9d4e59b3d.tmp/sketch/sketch_nov10b.ino.cpp"
# 1 "<command-line>"
# 1 "/tmp/buildfc9ac49b4389b81833fcbab9d4e59b3d.tmp/sketch/sketch_nov10b.ino.cpp"
#include "foo.h"
# 1 "/tmp/buildfc9ac49b4389b81833fcbab9d4e59b3d.tmp/sketch/sketch_nov10b.ino.cpp"
# 1 "/tmp/buildfc9ac49b4389b81833fcbab9d4e59b3d.tmp/sketch/foo.h" 1
# 2 "/tmp/buildfc9ac49b4389b81833fcbab9d4e59b3d.tmp/sketch/sketch_nov10b.ino.cpp" 2
#include <Test.h>

This is from a file that first includes an empty foo.h and then a non-existent Test.h.

@matthijskooijman
Copy link
Collaborator

If we want to parse the error output, -fno-diagnostics-show-caret could be useful (which suppresses the source line display and the ^ line to indicate the error). However, the first fatal error should pretty much always on the first line (especially when also running with -w), so only looking at the first line might also be sufficient.

@ffissore ffissore modified the milestones: 1.1.4, 1.0.2 Nov 11, 2015
matthijskooijman added a commit to matthijskooijman/arduino-builder that referenced this issue May 18, 2016
This is old code that detected includes by letting gcc generate a
dependency file and parsing that. It has been shown this method does not
work (see arduino#33) and it was replaced by more robust methods since.

Fixes arduino#108

Signed-off-by: Matthijs Kooijman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants