jerryscript
jerryscript copied to clipboard
fix building with LINE_INFO
config.h has to be included first
JerryScript-DCO-1.0-Signed-off-by: Maciej Musiał [email protected]
Please do explain why it would be necessary to hoist the include out of the JERRY_LINE_INFO
guard. In which configuration does it cause any problems?
When you modify config.h so that JERRY_LINE_INFO is defined as 1 by default, the project no longer compiles.
I am pretty sure that the root cause for that is not in this file. It may be true that the ecma-globals.h
header is needed somewhere where it is not included, but moving the include in jerry-core/ecma/base/ecma-line-info.h
out of the guard is certainly a quick'n'dirty fix only.
If there was an issue report for the problem (with properly filled in details) then it were easier to reproduce the build fail and better see which files do need to be fixed actually.
It is generally considered good practice to make your includes independent of who is including them. It is not the case with ecma-line-info.h: ecma-line-info.h references JERRY_LINE_INFO before including "ecma-globals.h", which defines it. This makes whoever includes ecma-line-info.h responsible for including "ecma-globals.h" first. However, "emac-line-info.c", "js-parser-line-info-create.c" don't do that, and therefore they don't compile (all definitions from ecma-line-info.h are missing).
The project only compiles as is, if you define JERRY_LINE_INFO as a global compiler switch with command line argument -D.
Well, right. So, the problem is that config.h
is not included before the guard. And the other problem is that the project rarely includes config.h
directly, but often lazily relies on ecma-globals.h
to include config.h
transitively.
I am not totally sure which way to go. A) Include config.h
before the guard and ecma-globals.h
within the guard, which is the "proper" way, but not really used consistently throughout the code base. B) Or just be sloppy like the rest of the code and include ecma-globals.h
before the guard even if we only need config.h
.
@zherczeg ? others?