Skip to content

[clang][DependencyScanning] Add API to expose module CompilerInvocation #10667

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

Open
wants to merge 1 commit into
base: stable/20240723
Choose a base branch
from

Conversation

cachemeifyoucan
Copy link

@cachemeifyoucan cachemeifyoucan commented May 12, 2025

Add a new API that returns underlying CompilerInvocation from ModuleDep. This allows clients to inspect and modify the state inside CompilerInvocation before turning that into build arguments.

@cachemeifyoucan cachemeifyoucan requested a review from a team as a code owner May 12, 2025 18:41
@jansvoboda11
Copy link

Why don't we just expose the underlying CompilerInvocation inside ModuleDeps to Swift, if it's currently happy modifying it after the Clang scan finishes?

@cachemeifyoucan
Copy link
Author

Why don't we just expose the underlying CompilerInvocation inside ModuleDeps to Swift, if it's currently happy modifying it after the Clang scan finishes?

Isn't this patch just did something similar? Just need an entry point to modified the underlying CompilerInvocation before calling getBuildArguments(). Register in the service is less likely to cause error due to calling order of the function of conflicting callback being applied cause the cached build command out of sync. I am definitely up for better alternative if it can make API clearer.

@jansvoboda11
Copy link

Why don't we just expose the underlying CompilerInvocation inside ModuleDeps to Swift, if it's currently happy modifying it after the Clang scan finishes?

Isn't this patch just did something similar? Just need an entry point to modified the underlying CompilerInvocation before calling getBuildArguments(). Register in the service is less likely to cause error due to calling order of the function of conflicting callback being applied cause the cached build command out of sync. I am definitely up for better alternative if it can make API clearer.

I find that post-processing value-type results is much clearer compared to callbacks. I also don't think the current implementation of the callback is sound due to the ordering I mentioned in my inlined comment. If that ordering is indeed necessary, I'd prefer to delegate that to the client rather than bake that into the Clang scanner API.

@cachemeifyoucan
Copy link
Author

If that ordering is indeed necessary, I'd prefer to delegate that to the client rather than bake that into the Clang scanner API.

Ordering is still not controlled by client if you expose internal compiler invocation because you cannot modify the invocation where you mentioned in the inline comment. You can only do it afterwards. But I am fine with adding that flexibility.

Add a new API that returns underlying CompilerInvocation from ModuleDep.
This allows clients to inspect and modify the state inside
CompilerInvocation before turning that into build arguments.
@cachemeifyoucan cachemeifyoucan force-pushed the clang-scanner-invocation-callback branch from 217a752 to cb2b900 Compare May 13, 2025 22:53
@cachemeifyoucan cachemeifyoucan changed the title [clang][DependencyScanning] Add API to register callback for cmd transform [clang][DependencyScanning] Add API to expose module CompilerInvocation May 13, 2025
@cachemeifyoucan
Copy link
Author

@swift-ci please test

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