Skip to content

[ClangImporter] Put the Swift compiler version into the module hash #17344

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 21, 2018
Merged

[ClangImporter] Put the Swift compiler version into the module hash #17344

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/ClangImporter/SwiftLookupTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1606,7 +1606,8 @@ SwiftNameLookupExtension::hashExtension(llvm::hash_code code) const {
return llvm::hash_combine(code, StringRef("swift.lookup"),
SWIFT_LOOKUP_TABLE_VERSION_MAJOR,
SWIFT_LOOKUP_TABLE_VERSION_MINOR,
inferImportAsMember);
inferImportAsMember,
version::getSwiftFullVersion());
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have expected this to match what we add to metadata.UserInfo above - should this use swiftCtx.LangOpts.EffectiveLanguageVersion or maybe the user info should remove it? Apropos, should there be a test that checks that output using clang -module-file-info?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The effective language version already makes its way into the hash via API notes, but if you think it's simpler to just have them do the same thing I can change it. I don't see how I can write a test for the hash changing, though, since this one's about what happens if you change how the compiler is built.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, now I see that setting the effective version just makes it print a superset of the version information, not different information. Nevermind, it's fine as-is.

I don't see how I can write a test for the hash changing

I agree, you can't really test the hashing. I was thinking it would be nice to test what we're printing with clang -module-file-info for the extension, since the principle is that anything affecting the hash should get printed there. But that's not something you need to do for this patch, sorry for not making that clear.

}

void importer::addEntryToLookupTable(SwiftLookupTable &table,
Expand Down