Skip to content
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

Fix equality implementation of module UID. #1814

Merged
merged 4 commits into from
Jul 31, 2024
Merged

Conversation

bbannier
Copy link
Member

We already computed a unique ID value for each module to allow
declaring the same ID name multiple times; we however did not
consistently use that value in the implementation of module::UID
equality and hash operators which is addressed by this patch.

Closes #1813.

@bbannier bbannier self-assigned this Jul 30, 2024
@bbannier bbannier marked this pull request as ready for review July 30, 2024 08:27
@bbannier bbannier requested a review from rsmmr July 30, 2024 08:27
rsmmr
rsmmr previously approved these changes Jul 30, 2024
@bbannier bbannier force-pushed the topic/bbannier/issue-1813 branch 2 times, most recently from ba23efa to 36589d5 Compare July 30, 2024 13:28
@bbannier bbannier requested a review from rsmmr July 30, 2024 14:01
@bbannier bbannier force-pushed the topic/bbannier/issue-1813 branch from 36589d5 to d73a18e Compare July 30, 2024 15:40
@bbannier bbannier requested a review from rsmmr July 30, 2024 15:41
rsmmr
rsmmr previously approved these changes Jul 31, 2024
Copy link
Member

@rsmmr rsmmr left a comment

Choose a reason for hiding this comment

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

Thanks!

bbannier added 4 commits July 31, 2024 08:50
We already computed a `unique` `ID` value for each module to allow
declaring the same `ID` name multiple times; we however did not
consistently use that value in the implementation of `module::UID`
equality and hash operators which is addressed by this patch.

Closes #1813.
We did this previously but stopped doing it with #1462.
While we ignore duplicate files it was still possible to erroneously add
the same file multiple times to a compilation. Catch this trivial case.
@bbannier bbannier force-pushed the topic/bbannier/issue-1813 branch from d73a18e to 3985ae7 Compare July 31, 2024 08:18
@bbannier bbannier merged commit a86b225 into main Jul 31, 2024
19 checks passed
@bbannier bbannier deleted the topic/bbannier/issue-1813 branch July 31, 2024 09:37
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.

Assertion failure if same module included in compilation twice
2 participants