Use bool in place of int for boolean things in the C API

[GeorgeLyon]

Summary

bool is pretty well supported by now in C, and using it in place of int is not only more semantically accurate, but also improves automatic bindings for languages like Swift.

There is more discussion here: https://llvm.discourse.group/t/adding-mlirbool-to-c-bindings/2280/5

[mehdi_amini]

If this builds with MSVC nowadays, then LGTM :)

[ftynse]

Could you write a very brief summary of the discussion in the commit message? Links tend to rot unlike git history.

[GeorgeLyon]

I'm guessing it is safe to ignore the unused function warnings? Also, It looks like the build failure was unrelated to the code change... is there any way to re-run a build?

[mehdi_amini]

In D92193#2420724, @GeorgeLyon wrote:

I'm guessing it is safe to ignore the unused function warnings?

Yes

Also, It looks like the build failure was unrelated to the code change... is there any way to re-run a build?

Rebase and update the diff

[GeorgeLyon]

Rebasing to re-run CI

[GeorgeLyon]

Let's give Debian another chance...

[GeorgeLyon]

I keep getting this error saying that the Debian build failed, though I don't see any errors:

Testing Time: 121.11s
  Unsupported      :   584
  Passed           : 40988
  Expectedly Failed:   174

I do see some clang-tidy _warnings_: clang-tidy found 0 errors and 9 warnings. 9 of them were added as review comments, though the CI indicates that it failed. Is this just "warnings are failures but its OK to ignore" or is something more nefarious happening here?

Also, why are the isNull methods static inline while the isEqual methods are regular methods? Maybe the correct thing to do here is to just make them all regular (non-static) methods?

[mehdi_amini]

I do see some clang-tidy _warnings_: clang-tidy found 0 errors and 9 warnings. 9 of them were added as review comments, though the CI indicates that it failed. Is this just "warnings are failures but its OK to ignore" or is something more nefarious happening here?

This is fine to ignore after you check the warnings.

Also, why are the isNull methods static inline while the isEqual methods are regular methods? Maybe the correct thing to do here is to just make them all regular (non-static) methods?

Only the helpers that are defined in the header are static inline. I think that the intent was to not exposing them in libMLIR.so and have them inlined in the client. I am not sure this is valid in C though: they has to be an external definition I believe and this would break if a client was taking the address.

[GeorgeLyon]

Only the helpers that are defined in the header are static inline. I think that the intent was to not exposing them in libMLIR.so and have them inlined in the client. I am not sure this is valid in C though: they has to be an external definition I believe and this would break if a client was taking the address.

Sorry if it was unclear, but my suggestion was to define the …isNull functions in the source. I'm not sure why you would want to avoid exposing …isNull in libMLIR.so but be fine with exposing …isEqual... the two seem to have similar levels of "helperiness".

mlir/include/mlir-c/IR.h

 // with C++ due to name mangling. So in addition to C, this interface enables
 // tools written in such languages.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_C_IR_H
 #define MLIR_C_IR_H
 
+#include <stdbool.h>
 #include <stdint.h>
 
 #include "mlir-c/Support.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 

 //===----------------------------------------------------------------------===//
 // Context API.
 //===----------------------------------------------------------------------===//
 
 /// Creates an MLIR context and transfers its ownership to the caller.
 MLIR_CAPI_EXPORTED MlirContext mlirContextCreate();
 
 /// Checks if two contexts are equal.
-MLIR_CAPI_EXPORTED int mlirContextEqual(MlirContext ctx1, MlirContext ctx2);
+MLIR_CAPI_EXPORTED bool mlirContextEqual(MlirContext ctx1, MlirContext ctx2);
 
 /// Checks whether a context is null.
-static inline int mlirContextIsNull(MlirContext context) {
+static inline bool mlirContextIsNull(MlirContext context) {
   return !context.ptr;
 }
 
 /// Takes an MLIR context owned by the caller and destroys it.
 MLIR_CAPI_EXPORTED void mlirContextDestroy(MlirContext context);
 
 /// Sets whether unregistered dialects are allowed in this context.
 MLIR_CAPI_EXPORTED void

 //===----------------------------------------------------------------------===//
 // Dialect API.
 //===----------------------------------------------------------------------===//
 
 /// Returns the context that owns the dialect.
 MLIR_CAPI_EXPORTED MlirContext mlirDialectGetContext(MlirDialect dialect);
 
 /// Checks if the dialect is null.
-static inline int mlirDialectIsNull(MlirDialect dialect) {
+static inline bool mlirDialectIsNull(MlirDialect dialect) {
   return !dialect.ptr;
 }
 
 /** Checks if two dialects that belong to the same context are equal. Dialects
  * from different contexts will not compare equal. */
-MLIR_CAPI_EXPORTED int mlirDialectEqual(MlirDialect dialect1,
+MLIR_CAPI_EXPORTED bool mlirDialectEqual(MlirDialect dialect1,
                                          MlirDialect dialect2);
 
 /// Returns the namespace of the given dialect.
 MLIR_CAPI_EXPORTED MlirStringRef mlirDialectGetNamespace(MlirDialect dialect);
 
 //===----------------------------------------------------------------------===//
 // Location API.
 //===----------------------------------------------------------------------===//
 

 
 /// Gets the context that a module was created with.
 MLIR_CAPI_EXPORTED MlirContext mlirModuleGetContext(MlirModule module);
 
 /// Gets the body of the module, i.e. the only block it contains.
 MLIR_CAPI_EXPORTED MlirBlock mlirModuleGetBody(MlirModule module);
 
 /// Checks whether a module is null.
-static inline int mlirModuleIsNull(MlirModule module) { return !module.ptr; }
+static inline bool mlirModuleIsNull(MlirModule module) { return !module.ptr; }
 
 /// Takes a module owned by the caller and deletes it.
 MLIR_CAPI_EXPORTED void mlirModuleDestroy(MlirModule module);
 
 /// Views the module as a generic operation.
 MLIR_CAPI_EXPORTED MlirOperation mlirModuleGetOperation(MlirModule module);
 
 //===----------------------------------------------------------------------===//

 /// Creates an operation and transfers ownership to the caller.
 MLIR_CAPI_EXPORTED MlirOperation
 mlirOperationCreate(const MlirOperationState *state);
 
 /// Takes an operation owned by the caller and destroys it.
 MLIR_CAPI_EXPORTED void mlirOperationDestroy(MlirOperation op);
 
 /// Checks whether the underlying operation is null.
-static inline int mlirOperationIsNull(MlirOperation op) { return !op.ptr; }
+static inline bool mlirOperationIsNull(MlirOperation op) { return !op.ptr; }
 
 /** Checks whether two operation handles point to the same operation. This does
  * not perform deep comparison. */
-MLIR_CAPI_EXPORTED int mlirOperationEqual(MlirOperation op,
+MLIR_CAPI_EXPORTED bool mlirOperationEqual(MlirOperation op,
                                            MlirOperation other);
 
 /// Gets the name of the operation as an identifier.
 MLIR_CAPI_EXPORTED MlirIdentifier mlirOperationGetName(MlirOperation op);
 
 /** Gets the block that owns this operation, returning null if the operation is
  * not owned. */
 MLIR_CAPI_EXPORTED MlirBlock mlirOperationGetBlock(MlirOperation op);
 

 
 /// Creates a new empty region and transfers ownership to the caller.
 MLIR_CAPI_EXPORTED MlirRegion mlirRegionCreate();
 
 /// Takes a region owned by the caller and destroys it.
 MLIR_CAPI_EXPORTED void mlirRegionDestroy(MlirRegion region);
 
 /// Checks whether a region is null.
-static inline int mlirRegionIsNull(MlirRegion region) { return !region.ptr; }
+static inline bool mlirRegionIsNull(MlirRegion region) { return !region.ptr; }
 
 /// Gets the first block in the region.
 MLIR_CAPI_EXPORTED MlirBlock mlirRegionGetFirstBlock(MlirRegion region);
 
 /// Takes a block owned by the caller and appends it to the given region.
 MLIR_CAPI_EXPORTED void mlirRegionAppendOwnedBlock(MlirRegion region,
                                                    MlirBlock block);
 

  * ownership to the caller. */
 MLIR_CAPI_EXPORTED MlirBlock mlirBlockCreate(intptr_t nArgs,
                                              MlirType const *args);
 
 /// Takes a block owned by the caller and destroys it.
 MLIR_CAPI_EXPORTED void mlirBlockDestroy(MlirBlock block);
 
 /// Checks whether a block is null.
-static inline int mlirBlockIsNull(MlirBlock block) { return !block.ptr; }
+static inline bool mlirBlockIsNull(MlirBlock block) { return !block.ptr; }
 
 /** Checks whether two blocks handles point to the same block. This does not
  * perform deep comparison. */
-MLIR_CAPI_EXPORTED int mlirBlockEqual(MlirBlock block, MlirBlock other);
+MLIR_CAPI_EXPORTED bool mlirBlockEqual(MlirBlock block, MlirBlock other);
 
 /** Returns the block immediately following the given block in its parent
  * region. */
 MLIR_CAPI_EXPORTED MlirBlock mlirBlockGetNextInRegion(MlirBlock block);
 
 /// Returns the first operation in the block.
 MLIR_CAPI_EXPORTED MlirOperation mlirBlockGetFirstOperation(MlirBlock block);
 

 MLIR_CAPI_EXPORTED void
 mlirBlockPrint(MlirBlock block, MlirStringCallback callback, void *userData);
 
 //===----------------------------------------------------------------------===//
 // Value API.
 //===----------------------------------------------------------------------===//
 
 /// Returns whether the value is null.
-static inline int mlirValueIsNull(MlirValue value) { return !value.ptr; }
+static inline bool mlirValueIsNull(MlirValue value) { return !value.ptr; }
 
 /// Returns 1 if two values are equal, 0 otherwise.
-int mlirValueEqual(MlirValue value1, MlirValue value2);
+bool mlirValueEqual(MlirValue value1, MlirValue value2);
 
 /// Returns 1 if the value is a block argument, 0 otherwise.
 MLIR_CAPI_EXPORTED int mlirValueIsABlockArgument(MlirValue value);
 
 /// Returns 1 if the value is an operation result, 0 otherwise.
 MLIR_CAPI_EXPORTED int mlirValueIsAOpResult(MlirValue value);
 
 /** Returns the block in which this value is defined as an argument. Asserts if

 /// Parses a type. The type is owned by the context.
 MLIR_CAPI_EXPORTED MlirType mlirTypeParseGet(MlirContext context,
                                              MlirStringRef type);
 
 /// Gets the context that a type was created with.
 MLIR_CAPI_EXPORTED MlirContext mlirTypeGetContext(MlirType type);
 
 /// Checks whether a type is null.
-static inline int mlirTypeIsNull(MlirType type) { return !type.ptr; }
+static inline bool mlirTypeIsNull(MlirType type) { return !type.ptr; }
 
 /// Checks if two types are equal.
-MLIR_CAPI_EXPORTED int mlirTypeEqual(MlirType t1, MlirType t2);
+MLIR_CAPI_EXPORTED bool mlirTypeEqual(MlirType t1, MlirType t2);
 
 /** Prints a location by sending chunks of the string representation and
  * forwarding `userData to `callback`. Note that the callback may be called
  * several times with consecutive chunks of the string. */
 MLIR_CAPI_EXPORTED void
 mlirTypePrint(MlirType type, MlirStringCallback callback, void *userData);
 
 /// Prints the type to the standard error stream.

 
 /// Gets the context that an attribute was created with.
 MLIR_CAPI_EXPORTED MlirContext mlirAttributeGetContext(MlirAttribute attribute);
 
 /// Gets the type of this attribute.
 MLIR_CAPI_EXPORTED MlirType mlirAttributeGetType(MlirAttribute attribute);
 
 /// Checks whether an attribute is null.
-static inline int mlirAttributeIsNull(MlirAttribute attr) { return !attr.ptr; }
+static inline bool mlirAttributeIsNull(MlirAttribute attr) { return !attr.ptr; }
 
 /// Checks if two attributes are equal.
-MLIR_CAPI_EXPORTED int mlirAttributeEqual(MlirAttribute a1, MlirAttribute a2);
+MLIR_CAPI_EXPORTED bool mlirAttributeEqual(MlirAttribute a1, MlirAttribute a2);
 
 /** Prints an attribute by sending chunks of the string representation and
  * forwarding `userData to `callback`. Note that the callback may be called
  * several times with consecutive chunks of the string. */
 MLIR_CAPI_EXPORTED void mlirAttributePrint(MlirAttribute attr,
                                            MlirStringCallback callback,
                                            void *userData);
 
 /// Prints the attribute to the standard error stream.
 MLIR_CAPI_EXPORTED void mlirAttributeDump(MlirAttribute attr);
 
 /// Associates an attribute with the name. Takes ownership of neither.
 MLIR_CAPI_EXPORTED MlirNamedAttribute mlirNamedAttributeGet(MlirStringRef name,
                                                             MlirAttribute attr);
 
 //===----------------------------------------------------------------------===//
 // Identifier API.
 //===----------------------------------------------------------------------===//
 
 /// Gets an identifier with the given string value.
 MLIR_CAPI_EXPORTED MlirIdentifier mlirIdentifierGet(MlirContext context,
                                                     MlirStringRef str);
 
 /// Checks whether two identifiers are the same.
-MLIR_CAPI_EXPORTED int mlirIdentifierEqual(MlirIdentifier ident,
+MLIR_CAPI_EXPORTED bool mlirIdentifierEqual(MlirIdentifier ident,
                                             MlirIdentifier other);
 
 /// Gets the string value of the identifier.
 MLIR_CAPI_EXPORTED MlirStringRef mlirIdentifierStr(MlirIdentifier ident);
 
 #ifdef __cplusplus
 }
 #endif
 
 #endif // MLIR_C_IR_H

mlir/lib/CAPI/IR/IR.cpp

 // Context API.
 //===----------------------------------------------------------------------===//
 
 MlirContext mlirContextCreate() {
   auto *context = new MLIRContext;
   return wrap(context);
 }
 
-int mlirContextEqual(MlirContext ctx1, MlirContext ctx2) {
+bool mlirContextEqual(MlirContext ctx1, MlirContext ctx2) {
   return unwrap(ctx1) == unwrap(ctx2);
 }
 
 void mlirContextDestroy(MlirContext context) { delete unwrap(context); }
 
 void mlirContextSetAllowUnregisteredDialects(MlirContext context, int allow) {
   unwrap(context)->allowUnregisteredDialects(allow);
 }

 //===----------------------------------------------------------------------===//
 // Dialect API.
 //===----------------------------------------------------------------------===//
 
 MlirContext mlirDialectGetContext(MlirDialect dialect) {
   return wrap(unwrap(dialect)->getContext());
 }
 
-int mlirDialectEqual(MlirDialect dialect1, MlirDialect dialect2) {
+bool mlirDialectEqual(MlirDialect dialect1, MlirDialect dialect2) {
   return unwrap(dialect1) == unwrap(dialect2);
 }
 
 MlirStringRef mlirDialectGetNamespace(MlirDialect dialect) {
   return wrap(unwrap(dialect)->getNamespace());
 }
 
 //===----------------------------------------------------------------------===//
MlirOperation mlirOperationCreate(const MlirOperationState *state) {
   free(state->successors);
   free(state->regions);
   free(state->attributes);
   return result;
 }
 
 void mlirOperationDestroy(MlirOperation op) { unwrap(op)->erase(); }
 
-int mlirOperationEqual(MlirOperation op, MlirOperation other) {
+bool mlirOperationEqual(MlirOperation op, MlirOperation other) {
   return unwrap(op) == unwrap(other);
 }
 
 MlirIdentifier mlirOperationGetName(MlirOperation op) {
   return wrap(unwrap(op)->getName().getIdentifier());
 }
 
 MlirBlock mlirOperationGetBlock(MlirOperation op) {

 
 MlirBlock mlirBlockCreate(intptr_t nArgs, MlirType const *args) {
   Block *b = new Block;
   for (intptr_t i = 0; i < nArgs; ++i)
     b->addArgument(unwrap(args[i]));
   return wrap(b);
 }
 
-int mlirBlockEqual(MlirBlock block, MlirBlock other) {
+bool mlirBlockEqual(MlirBlock block, MlirBlock other) {
   return unwrap(block) == unwrap(other);
 }
 
 MlirBlock mlirBlockGetNextInRegion(MlirBlock block) {
   return wrap(unwrap(block)->getNextNode());
 }
 
 MlirOperation mlirBlockGetFirstOperation(MlirBlock block) {
void mlirBlockPrint(MlirBlock block, MlirStringCallback callback,
   detail::CallbackOstream stream(callback, userData);
   unwrap(block)->print(stream);
 }
 
 //===----------------------------------------------------------------------===//
 // Value API.
 //===----------------------------------------------------------------------===//
 
-int mlirValueEqual(MlirValue value1, MlirValue value2) {
+bool mlirValueEqual(MlirValue value1, MlirValue value2) {
   return unwrap(value1) == unwrap(value2);
 }
 
 int mlirValueIsABlockArgument(MlirValue value) {
   return unwrap(value).isa<BlockArgument>();
 }
 
 int mlirValueIsAOpResult(MlirValue value) {

 MlirType mlirTypeParseGet(MlirContext context, MlirStringRef type) {
   return wrap(mlir::parseType(unwrap(type), unwrap(context)));
 }
 
 MlirContext mlirTypeGetContext(MlirType type) {
   return wrap(unwrap(type).getContext());
 }
 
-int mlirTypeEqual(MlirType t1, MlirType t2) { return unwrap(t1) == unwrap(t2); }
+bool mlirTypeEqual(MlirType t1, MlirType t2) {
+  return unwrap(t1) == unwrap(t2);
+}
 
 void mlirTypePrint(MlirType type, MlirStringCallback callback, void *userData) {
   detail::CallbackOstream stream(callback, userData);
   unwrap(type).print(stream);
 }
 
 void mlirTypeDump(MlirType type) { unwrap(type).dump(); }
 
 //===----------------------------------------------------------------------===//
 // Attribute API.
 //===----------------------------------------------------------------------===//
 
 MlirAttribute mlirAttributeParseGet(MlirContext context, MlirStringRef attr) {
   return wrap(mlir::parseAttribute(unwrap(attr), unwrap(context)));
 }
 
 MlirContext mlirAttributeGetContext(MlirAttribute attribute) {
   return wrap(unwrap(attribute).getContext());
 }
 
 MlirType mlirAttributeGetType(MlirAttribute attribute) {
   return wrap(unwrap(attribute).getType());
 }
 
-int mlirAttributeEqual(MlirAttribute a1, MlirAttribute a2) {
+bool mlirAttributeEqual(MlirAttribute a1, MlirAttribute a2) {
   return unwrap(a1) == unwrap(a2);
 }
 
 void mlirAttributePrint(MlirAttribute attr, MlirStringCallback callback,
                         void *userData) {
   detail::CallbackOstream stream(callback, userData);
   unwrap(attr).print(stream);
 }
 
 void mlirAttributeDump(MlirAttribute attr) { unwrap(attr).dump(); }
 
 MlirNamedAttribute mlirNamedAttributeGet(MlirStringRef name,
                                          MlirAttribute attr) {
   return MlirNamedAttribute{name, attr};
 }
 
 //===----------------------------------------------------------------------===//
 // Identifier API.
 //===----------------------------------------------------------------------===//
 
 MlirIdentifier mlirIdentifierGet(MlirContext context, MlirStringRef str) {
   return wrap(Identifier::get(unwrap(str), unwrap(context)));
 }
 
-int mlirIdentifierEqual(MlirIdentifier ident, MlirIdentifier other) {
+bool mlirIdentifierEqual(MlirIdentifier ident, MlirIdentifier other) {
   return unwrap(ident) == unwrap(other);
 }
 
 MlirStringRef mlirIdentifierStr(MlirIdentifier ident) {
   return wrap(unwrap(ident).strref());
 }