Skip to content

[clang][Modules] Adding C-API for Negative Stat Caching Diagnostics #10524

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 8 commits into
base: next
Choose a base branch
from

Conversation

qiongsiwu
Copy link

llvm#135703 added a C++ API to the shared cached to diagnose invalid negatively stat cached paths. This PR adds a C API so an external system can take advantage of the diagnostics.

@qiongsiwu
Copy link
Author

qiongsiwu commented Apr 22, 2025

Note to reviewers:
The test added here is simply testing that the API is called correctly. llvm#135703 has a test to test for correctly producing the paths. I am not aware of a simple way to test the C-API for its content (i.e. creating a scan where some file s created in the middle to invalidate the negative stat cache). If you are aware of such tests, I am all ears.

Thanks!

@qiongsiwu qiongsiwu requested review from Bigcheese and jansvoboda11 and removed request for Bigcheese April 22, 2025 20:03
@qiongsiwu qiongsiwu requested a review from jansvoboda11 April 24, 2025 17:07
@@ -585,6 +585,11 @@ const char *clang_experimental_DepGraph_getTUContextHash(CXDepGraph);
CINDEX_LINKAGE
CXDiagnosticSet clang_experimental_DepGraph_getDiagnostics(CXDepGraph);

CINDEX_LINKAGE
CXStringSet *

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only now do I see that we use CXCStringArray everywhere in the scanner API. This means that introducing new CXStringSet type will impose additional burden on the client. Unlike upstream, we already have factory functions that create reference-like CXCStringArray here in the next branch. Can we switch to that type instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! The code is updated. Thanks for taking a careful look.

I don't particularly like the code I have for createCStringsRefImpl. It feels clumsy. I have to have the lambda to handle the simple task of getting the start of the string data. I am all ears if there are simpler ways to do this.

@qiongsiwu qiongsiwu requested a review from jansvoboda11 April 24, 2025 20:58
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