Conversation
…VM 22 LLVM 22 deprecated 8 global-context C API functions in favor of their context-aware counterparts. Instead of suppressing the warnings, call the non-deprecated InContext APIs directly. The non-context Go functions (MDKindID, StructType, ConstString, ConstStruct, AddBasicBlock, InsertBasicBlock) now delegate to GlobalContext().<method>(), which already calls the InContext C APIs. IntPtrType calls LLVMIntPtrTypeInContext directly. Only LLVMGetGlobalContext still needs a warning-suppression wrapper in deprecated.c since it has no non-deprecated replacement. The context-aware C APIs have been available since well before LLVM 14, so this works across all supported LLVM versions. Signed-off-by: deadprogram <ron@hybridgroup.com>
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Wrap deprecated LLVMGetGlobalContext + delegation refactor
Overall: Well-scoped, high-quality change. The deprecated.c/deprecated.h shim cleanly isolates the single deprecated LLVM 22 call (LLVMGetGlobalContext) that has no non-deprecated replacement, using a properly balanced #pragma GCC diagnostic push/pop so the suppression doesn't leak. The ir.go refactor centralizes global-context logic by delegating the package-level functions (MDKindID, StructType, ConstString, ConstStruct, AddBasicBlock, InsertBasicBlock) to their Context-receiver counterparts.
Verified:
- Delegation is behaviorally equivalent to the removed inline implementations — the
...InContextvariants with the global context match what the old non-context C APIs did internally.target.goIntPtrTypeviaLLVMIntPtrTypeInContextlikewise matches the deprecatedLLVMIntPtrType. - No leftover references to the raw deprecated symbol outside the wrapper; both
ir.goandtarget.goincludedeprecated.h. - cgo
C.CString/defer C.freehandling and header guards/extern "C"are correct. No memory-safety or performance concerns introduced (the added Go indirection is on cold IR-construction paths; cgo crossing count is unchanged).
Two minor, non-blocking nits are noted inline.
Findings without inline locations
target.go:148: Idiom consistency nit: this site callsC.LLVMGetGlobalContext_wrap()directly, whereas their.gorefactor routes everything through theGlobalContext()helper. For a single point of truth, considert.C = C.LLVMIntPtrTypeInContext(GlobalContext().C, td.C). Purely stylistic — not a defect.
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #ifndef LLVM_BINDINGS_GO_LLVM_DEPRECATED_H | ||
| #define LLVM_BINDINGS_GO_LLVM_DEPRECATED_H |
There was a problem hiding this comment.
Minor documentation nit: "All other wrapped functions are handled by calling their context-aware counterparts from Go" is a bit misleading — this file wraps exactly one function (LLVMGetGlobalContext_wrap) and contains no other wrappers. The intent is that other deprecated context-less C APIs are avoided by calling their ...InContext counterparts directly from Go, so no additional wrappers are needed here. Consider rewording, e.g.: "The other deprecated context-less C APIs are avoided by calling their context-aware counterparts directly from Go, so no additional wrappers are needed." (Same text in deprecated.c.)
2 participants
LLVM 22 deprecated 8 global-context C API functions in favor of their context-aware counterparts. Instead of suppressing the warnings, call the non-deprecated InContext APIs directly.
The non-context Go functions (MDKindID, StructType, ConstString, ConstStruct, AddBasicBlock, InsertBasicBlock) now delegate to GlobalContext().(), which already calls the InContext C APIs. IntPtrType calls LLVMIntPtrTypeInContext directly.
Only LLVMGetGlobalContext still needs a warning-suppression wrapper in deprecated.c since it has no non-deprecated replacement.
The context-aware C APIs have been available since well before LLVM 14, so this works across all supported LLVM versions.