This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Define ABI breaking class members correctly
ClosedPublic

Authored by kovdan01 on Mar 13 2022, 5:01 AM.

Details

Summary

Non-static class members declared under #ifndef NDEBUG should be declared
under #if LLVM_ENABLE_ABI_BREAKING_CHECKS to make headers library-friendly and
allow cross-linking, as discussed in D120714.

Diff Detail

Event Timeline

kovdan01 created this revision.Mar 13 2022, 5:01 AM
Herald added a project: Restricted Project. · View Herald Transcript
kovdan01 requested review of this revision.Mar 13 2022, 5:01 AM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptMar 13 2022, 5:01 AM
lebedev.ri edited the summary of this revision. (Show Details)Mar 13 2022, 5:14 AM
mehdi_amini added inline comments.Mar 13 2022, 10:56 AM
mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
601 ↗(On Diff #414929)

This is a private header (you're in the lib directory and not in the include directory)

Same for the SystemZ one above I believe.

kovdan01 updated this revision to Diff 415022.Mar 14 2022, 12:57 AM

Keep private headers untouched

kovdan01 marked an inline comment as done.Mar 14 2022, 12:58 AM
kovdan01 added inline comments.
mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
601 ↗(On Diff #414929)

Makes sense, fixed, thanks!

This revision is now accepted and ready to land.Mar 14 2022, 1:04 AM
kovdan01 updated this revision to Diff 415049.Mar 14 2022, 4:00 AM
kovdan01 marked an inline comment as done.
kovdan01 removed projects: Restricted Project, Restricted Project.

Fix formatting problems found on pre-merge checks

dexonsmith accepted this revision.Mar 14 2022, 11:09 AM

LGTM too; thanks!

This revision was landed with ongoing or failed builds.Mar 24 2022, 2:25 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2022, 2:25 AM
kovdan01 updated this revision to Diff 417858.Mar 24 2022, 2:50 AM
kovdan01 retitled this revision from Define ABI breaking class members correctly to [CodeGen] Define ABI breaking class members correctly.

Accidentally placed this revision ID in commit message related to other patch. Reverted that.

This revision was landed with ongoing or failed builds.Mar 24 2022, 2:55 AM