Skip to content
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

Make directory-level enabling recursive #122

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

FeignClaims
Copy link

@FeignClaims FeignClaims commented Oct 11, 2023

Currently when enabling a directory by setting CPPFRONT_NO_MAGIC to false, only targets right in CMAKE_CURRENT_SOURCE_DIR will get automatic cpp2-to-cpp translation.

This conflict with the behavior that find_package makes the package available for all subdirectories inside the directory. That is, if users already call find_package in src, they are not required to find_package again in src/app.

The pr fix the above conflict by

  1. Making the directory enabling (cppfront_enable_directories) recursive.
  2. Making the paths of generated cpp files consistent with the origin cpp2 files. For instance, ${CMAKE_SOURCE_DIR}/app/src/main.cpp2 to ${CMAKE_BINARY_DIR}/_cppfront/app/src/main.cpp.

The second change enables users to navigate the corresponding cpp files. I've tested that the cpp2 file target_source-ed by multiple targets won't translate more than once, so I think this change is ok compared to the hash-and-rename.


A better solution might be making the cpp2-to-cpp translation registration behave like handling uic automatically for cmake-qt, but I failed to learn how that works.

@alexreinking
Copy link
Contributor

Thanks for your contribution! I will look at it and review this weekend.

@FeignClaims
Copy link
Author

Hi @alexreinking, just wanted to bring attention to this PR. Would you mind taking a look when you have a chance? Thanks!

@alexreinking
Copy link
Contributor

We're going to need to think about how to make this work with CXX_MODULE and HEADER_SET file sets that specify a BASE_DIR. For sources in those properties, we need to

  1. Identify the base dir they are below. CMake enforces that base dirs do not contain each other, so there is a unique one.
  2. Add new base dirs that preserve the relative path of the generated file.

Basically, if someone has a base dir of include/project with files like include/project/component/header.h2, we need to make sure that we add a generated base dir that will still allow component/header.h to be included.

@FeignClaims
Copy link
Author

We're going to need to think about how to make this work with CXX_MODULE and HEADER_SET file sets that specify a BASE_DIR.

I see, but I'm afraid I won't have time to modify this PR in the coming month. Let's change this PR to a draft PR for now, and I'll set it to "Ready for review" once I've made the necessary changes.

@FeignClaims FeignClaims marked this pull request as draft June 19, 2024 15:08
@alexreinking
Copy link
Contributor

Oh boy and while we're working on total correctness, we need to make sure that translating generated cpp2 sources works correctly

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

Successfully merging this pull request may close these issues.

2 participants