jerryscript icon indicating copy to clipboard operation
jerryscript copied to clipboard

fix building with LINE_INFO

Open aksdfauytv opened this issue 1 year ago • 5 comments

config.h has to be included first

JerryScript-DCO-1.0-Signed-off-by: Maciej Musiał [email protected]

aksdfauytv avatar Mar 09 '23 10:03 aksdfauytv

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?

akosthekiss avatar Mar 09 '23 11:03 akosthekiss

When you modify config.h so that JERRY_LINE_INFO is defined as 1 by default, the project no longer compiles.

aksdfauytv avatar Mar 09 '23 12:03 aksdfauytv

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.

akosthekiss avatar Mar 09 '23 12:03 akosthekiss

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.

aksdfauytv avatar Mar 09 '23 12:03 aksdfauytv

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?

akosthekiss avatar Mar 09 '23 13:03 akosthekiss