-
Notifications
You must be signed in to change notification settings - Fork 258
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
[SUGGESTION] Only include cpp2regex.h
when @regex
is used to improve compile time
#1199
Comments
This is a good point. Detecting this and adding the include could be an easy fix. If this is not desired an other fix would be to split |
That's interesting... does that mean we could do both? Only conditionally include the runtime part for the lowered C++ code? |
When I was reviewing the PR I thought of this, but I didn't grok it well enough to know what parts are the metafunction-at-compile-time part and what part is the runtime-support-library part. I think a good first step would be to move the metafunction-only parts right into |
I performed the split in #1219. I also tried to add the functionality for adding the include on the fly. But my understanding of cppfront is too limited. Unlinke my MR for enabling metafunction to add code outside of the context (#809), this is not straight forward. @hsutter Can you have look or give me some pointers on how to do this? |
Circling back: This has now been fixed in #1219, thanks again @bluetarpmedia for pointing this out! "Don't pay for what you don't use" FTW. 👍 |
I have seen the added |
The generated
cpp2regex.h
file is ~170 KB and so I wondered what the impact on compile times are.With clang 18 on ubuntu, I tested with and without including
cpp2regex.h
from insidecpp2util.h
when compiling an emptymain
function lowered from Cpp2:With cpp2regex.h
Without cpp2regex.h
I repeated the test a few times and the results were relatively stable. The real (wall clock) time is approx 2X more when bringing in
cpp2regex.h
, so I wondered if cppfront can detect whether it's needed and then conditionally include it?The text was updated successfully, but these errors were encountered: