logological / gpp

GPP, a generic preprocessor
https://logological.org/gpp
GNU Lesser General Public License v3.0
192 stars 33 forks source link

Reason for MAXINCL == 10? #11

Closed gavanderhoorn closed 7 years ago

gavanderhoorn commented 7 years ago

I'm using gpp as part of a toolchain wrapper and the build files generated by that toolchain can easily setup an include path with > 10 include directories. As gpp has MAXINCL set to 10 (here), those builds fail to run properly.

10 seems like a somewhat arbitrary number, so I'm wondering what the rationale for it was/is: I don't want to change something without understanding the consequences, but from looking through the rest of code, it seems like upping that nr should just work.

Would this be correct?

logological commented 7 years ago

Just about any source processor with file inclusion imposes a (usually arbitrary) limit on the nesting level in order to prevent runaway recursion. The current level of 10 in GPP was inherited from the original author. I suspect that this was chosen arbitrarily and am not aware of anything in the code depending on it. If you want to change it for your own build, it should probably work. We could also consider increasing it in the official sources here.

From a quick informal survey, I see that GCC's preprocessor imposes an #include nesting limit of 200. The C standard requires at least 15 levels, and the C++ standard recommends 256. For Visual Studio 2012 the officially documented limit is 10. LaTeX currently allows 127 levels of \include. So maybe something in the range of 127 to 256 is a more reasonable limit nowadays.

gavanderhoorn commented 7 years ago

Does MAXINCL limit the nesting depth, or the maximum nr of include directories that can be passed to gpp? The constant name seems to imply the former, but the comment seems to suggest the latter:

define MAXINCL 10 /* max # of include dirs */

logological commented 7 years ago

Oh, I'm sorry—what a silly mistake on my part. Serves me right for reading your question too fast! Yes, the vaguely named MAXINCL actually sets the maximum number of include directories that can be specified on the command line with -I. And no, it doesn't look like there's any reason that the value couldn't be increased. I think the only reason that the code uses a fixed-size array is that a dynamically allocated list is marginally more difficult to implement.

gavanderhoorn commented 7 years ago

Ah, ok. No problem. Just making sure I understand things correctly :).

Now to suggest a new arbitrary value: would something like 128 be acceptable? I realise it's just as arbitrary as 10 (although: it is a power of 2 ;)), and a list is probably better.

But this gives us some headroom without changing things too radically.

logological commented 7 years ago

Sure, that ought to be fine.

gavanderhoorn commented 7 years ago

Thanks.

That removes one more patch from my fork :)