Page MenuHomePhabricator

[LLVM-C] Add an accessor for the kind of a Metadata Node
ClosedPublic

Authored by CodaFi on Sep 29 2018, 8:37 AM.

Details

Summary

Allows for retrieving the type of a metadata node. Has the added benefit of ensuring that the C and C++ kind APIs stay in sync as a failure to add a corresponding LLVMMetadataKind will result in the switch in the accessor being semantically malformed.

Diff Detail

Repository
rL LLVM

Event Timeline

CodaFi created this revision.Sep 29 2018, 8:37 AM
whitequark requested changes to this revision.Sep 29 2018, 8:41 AM
whitequark added inline comments.
include/llvm-c/DebugInfo.h
1198 ↗(On Diff #167606)

C enums don't have a defined ABI and should never be used in function signatures, because this makes FFI impossible to do correctly without going through something libclang.

This revision now requires changes to proceed.Sep 29 2018, 8:41 AM
CodaFi added inline comments.Sep 29 2018, 8:47 AM
include/llvm-c/DebugInfo.h
1198 ↗(On Diff #167606)

So what's the workaround here? I recognize the ABI concerns given LLVM's stated policy on the C API, but I'm curious to know how this hasn't come up before considering Core.h and DebugInfo.h define and use over 20 different enumerations across their signatures.

whitequark added inline comments.Sep 29 2018, 8:49 AM
include/llvm-c/DebugInfo.h
1198 ↗(On Diff #167606)

Cast to an integer. I think to unsigned. All other uses of enumerations are a mistake as well; I'm not sure why they were introduced, but ideally they should be fixed.

CodaFi updated this revision to Diff 167627.Sep 29 2018, 2:24 PM
CodaFi marked 3 inline comments as done.
whitequark accepted this revision.Sep 29 2018, 3:34 PM

LGTM, thanks!

This revision is now accepted and ready to land.Sep 29 2018, 3:34 PM
This revision was automatically updated to reflect the committed changes.