[lldb] Fallback to implicit modules when explicit modules are missing #8356
Conversation
|
@artemcm do you know if it's possible to have a hybrid mode? By that I mean: Could we strip |
This is a good question for @jansvoboda11 or @Bigcheese . |
|
Clang would first parse the explicit module maps and then parse any additional module maps found during header search I think. The questions is: can you rely on the explicit module maps still being around if the PCM files aren't? And even if you check for that, is there any chance for time-of-check time-of-use bugs? |
| // The value's format is 'ModuleName=ModulePath'. Drop the prefix up to the | ||
| // first '=' character to obtain the path. |
There was a problem hiding this comment.
Not necessarily. There's another format that's just -fmodule-file=ModulePath we might switch to in the future.
There was a problem hiding this comment.
@jansvoboda11 thanks. Do you know, are explicit modules guaranteed to be absolute paths? I would presume yes.
There was a problem hiding this comment.
That ultimately depends on the build system. I'd expect them to be absolute in practice, but it's not a guarantee. Theoretically these might be relative to -working-directory or the process CWD.
In my local testing this was happening, causing duplicate/shadow issues. And so I removed the references to explicit module maps, and went full implicit. This resulted in compilation working. Thanks for explaining. |
Was the shadowing caused by two different module maps? I don't think Clang will parse the same module map twice. |
I don't think so, but I don't know. I could investigate further. What I can say for certain is that by removing |
|
@swift-ci test |
| // Check both path and value. This is to handle paths containing '='. | ||
| if (!llvm::sys::fs::exists(path) && !llvm::sys::fs::exists(value)) { | ||
| std::string m_description; | ||
| HEALTH_LOG_PRINTF("Nonexistent explicit module file %s", path.data()); |
There was a problem hiding this comment.
Unfortunately you'll need to either use path.str() or add a HEALTH_LOG macro that takes LLVM formats. There is no guarantee that .data() is nullterminated.
There was a problem hiding this comment.
The StringRef references a std::string, which I understand would it a guarantee that data is null terminated.
There was a problem hiding this comment.
- Technically correct :-)
- But then it would be safer to replace
const auto &withconst std::string &then to guard against future modification - It would be even better to add a HEALTH_LOG that uses llvm::format so we can avoid the problem altogether
There was a problem hiding this comment.
I'll do HEALTH_LOG in an isolated change.
There was a problem hiding this comment.
I replaced auto with std::string in 610d19f
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test windows |
|
@swift-ci test |
|
@swift-ci test windows |
|
@swift-ci test linux |
|
@swift-ci test |
kastiglione
deleted the
dl/lldb-Fallback-to-implicit-modules-when-explicit-modules-are-missing
branch
…#8356) Handle the case where an explicit module path does not exist, by falling back to implicit modules. The initial logic is: If any one explicit module is nonexistent, then remove all references to explicit modules. Additionally, remove all explicit module maps. (cherry-picked from commit 2abff47)
…#8356) (#8427) Handle the case where an explicit module path does not exist, by falling back to implicit modules. The initial logic is: If any one explicit module is nonexistent, then remove all references to explicit modules. Additionally, remove all explicit module maps. (cherry-picked from commit 2abff47)
Handle the case where an explicit module path does not exist, by falling back to implicit modules.
The initial logic is: If any one explicit module is nonexistent, then remove all references to explicit modules. Additionally, remove all explicit module maps.