Page MenuHomePhabricator

Use MlirStringRef throughout the C API
ClosedPublic

Authored by GeorgeLyon on Fri, Nov 20, 6:41 PM.

Details

Summary

While this makes the unit tests a bit more verbose, this simplifies the creation of bindings because only the bidirectional mapping between the host language's string type and MlirStringRef need to be implemented.

Diff Detail

Event Timeline

GeorgeLyon created this revision.Fri, Nov 20, 6:41 PM
GeorgeLyon requested review of this revision.Fri, Nov 20, 6:41 PM
GeorgeLyon edited the summary of this revision. (Show Details)Fri, Nov 20, 6:48 PM
GeorgeLyon added a reviewer: ftynse.

Removing spurious formatting changes

mehdi_amini added inline comments.Sat, Nov 21, 8:21 PM
mlir/include/mlir-c/IR.h
153

I rather keep the name mlirLocationFileLineColGet and not expose the const char * one: otherwise we'll have to "overload" every API with StringRef and char*

To be perfectly clear, I agree that we should use MlirStringRef throughout, but if we decide that we should do that across the API in one fell swoop (I’m happy to author that PR if folks are on board).

@ftynse thoughts?

Sure, let's use MlirStringRef everywhere, this also indicates that we don't take ownership of the string. We added it much later than APIs that could be using it and nobody had time to reconsider those yet.

Use MlirStringRef throughout the API.

Fix formatting

Use MlirStringRef for all strings in the C API

Use MlirStringRef for all strings in the C API

Use MlirStringRef throught the C API

GeorgeLyon retitled this revision from Allow Location to be created with a string ref to Use MlirStringRef throughout the C API.Mon, Nov 23, 11:48 AM
GeorgeLyon edited the summary of this revision. (Show Details)

Sorry for the spam, I am still learning my way around arc diff. This should be ready now though.

Harbormaster completed remote builds in B79836: Diff 307141.
Harbormaster completed remote builds in B79839: Diff 307144.

Some more formatting fixes

mehdi_amini accepted this revision.Mon, Nov 23, 12:46 PM
This revision is now accepted and ready to land.Mon, Nov 23, 12:46 PM
This revision was automatically updated to reflect the committed changes.

FYI: your email address wasn't picked up by git when landing - Author: George <>