This is an archive of the discontinued LLVM Phabricator instance.

[clang][deps] NFC: Remove dead code
ClosedPublic

Authored by jansvoboda11 on Jan 23 2023, 4:17 PM.

Details

Summary

This patch removes some dead code in the dependency scanner.

The ModuleDeps::ImplicitModulePCMPath member stopped being used in D131934.

The strict context hash was replaced in D129884 by hash of the canonical command line.

Diff Detail

Event Timeline

jansvoboda11 created this revision.Jan 23 2023, 4:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 23 2023, 4:17 PM
Herald added a subscriber: ributzka. · View Herald Transcript
jansvoboda11 requested review of this revision.Jan 23 2023, 4:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 23 2023, 4:17 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Removing ImplicitModulePCMPath LGTM; not sure about the other change.

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
180

I see we're enabling strict hashing in the scaner itself: ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true;, which makes me think this code was never used to influence the scanner's implicit build. If that's true, was this code *already* dead before my change in D129884? It's not clear to me what it was doing.

jansvoboda11 added inline comments.Jan 23 2023, 4:57 PM
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
180

The line you found enables strict context hashing for the CompilerInstance that performs the scanning build. That's still important so that we don't squash multiple module configurations into one minimized PCM.

The line this patch removes used to control generation of the module context hash we report to the client. This used to be done by taking the original TU command line, tweaking it, and calling CompilerInvocation::getModuleHash(). Since we now hash the whole command line that's generated from said CompilerInvocation, controlling the behavior of getModuleHash() by enabling strict context hash is no longer necessary.

benlangmuir accepted this revision.Jan 24 2023, 9:35 AM
benlangmuir added inline comments.
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
180

Got it, thanks for explaining!

This revision is now accepted and ready to land.Jan 24 2023, 9:35 AM
This revision was landed with ongoing or failed builds.Jan 24 2023, 9:50 AM
This revision was automatically updated to reflect the committed changes.