During Clang’s pre-processing/lexing phase, the directive dependency scanner ends up creating a new instance of LangOpts, instead of using the LangOpts from the CompilerInvocation. The new LangOpts instance does not respect the original LangOpts of the CompilerInvocation and is causing this bug - [clang-scan-deps] Error if integer literal contains a single quote in `#if` directive · Issue #88896 · llvm/llvm-project · GitHub that I want to put a fix out for.
This is the offending section: llvm-project/clang/lib/Lex/DependencyDirectivesScanner.cpp at 83646590afe222cfdd792514854549077e17b005 · llvm/llvm-project · GitHub and there are more details here:
[clang-scan-deps] Error if integer literal contains a single quote in `#if` directive · Issue #88896 · llvm/llvm-project · GitHub.
Looking to get some more help with this. Thanks!
Looks like Scanner is constructed in clang::scanSourceForDependencyDirectives so it mostly depends on whether clients of that API have a CompilerInvocation handy to pass through the right LangOpts.
I see the API used from lib/Frontend, PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction which has a CompilerInstance which has a (possibly null) handle on a CompilerInvocation.
It’s also used from lib/Tooling/DependencyScanning, DependencyScanningWorkerFilesystem::ensureDirectiveTokensArePopulated which is called by DependencyScanningAction::runInvocation which definitely has a CompilerInvocation.
Sounds like there are ways to get the correct LangOpts handed down? Caveat, I am in no way a frontend expert.
1 Like
@pogo59 Thanks for the pointers. These were my conclusions too, but there was one item that I want some advice on.
It’s also used from lib/Tooling/DependencyScanning, DependencyScanningWorkerFilesystem::ensureDirectiveTokensArePopulated which is called by DependencyScanningAction::runInvocation which definitely has a CompilerInvocation.
ensureDirectiveTokensArePopulated API is on the DependencyScanningWorkerFilesystem class, which has filesystem like APIs, and so I am not sure if an API signature like DependencyScanningWorkerFilesystem::ensureDirectiveTokensArePopulated( EntryRef Ref, const LangOptions &LangOpts) would be okay?
I am not familiar with the DependencyScanningWorkerFilesystem class, its boundaries/design etc., and so I wasn’t sure if we can modify its ensureDirectiveTokensArePopulated API to take in a LangOpts arg.
As in, would it look a bit off to someone if they were to look at the header for DependencyScanningWorkerFilesystem and see that the ensureDirectiveTokensArePopulated API took a LangOpts specifically.
Well, that would be up to reviewers to decide. If all you really need are language and dialect values, passing those down (instead of all of LangOpts) would be very reasonable IMO.
1 Like
Okay, thanks! I’ll try wiring it up to pass in LangOpts and then iterate based on the feedback I’d get.
If all you really need are language and dialect values,
I would need to pass LangOpts since Scanner constructs an independent instance of its own, which is disjoint from the actual LangOpts from CompilerInstance