This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Fix the names of exported functions
ClosedPublic

Authored by stella.stamenova on May 3 2022, 4:24 PM.

Details

Summary

The names of the functions that are supposed to be exported do not match the implementations. This is due in part to https://github.com/llvm/llvm-project/commit/cac7aabbd8236bef2909bfc0dbba17644f7aaade.

This change makes the implementations and declarations match and adds a couple missing declarations.

The new names follow the pattern of the existing verify functions where the prefix is maintained as _mlir_ciface_ but the suffix follows the new naming convention.

Diff Detail

Event Timeline

stella.stamenova requested review of this revision.May 3 2022, 4:24 PM

@mehdi_amini : This patch does not quite work because of the built-in calling convention (https://github.com/llvm/llvm-project-staging/blob/cc926dc3a87af7023aa9b6c392347a0a8ed6949b/mlir/docs/LLVMDialectMemRefConvention.md). Was the goal with the clang-tidy change to also change the convention? I could fix it the other way and undo the clang-tidy to these specific functions instead. This would be a smaller change.

I found this while trying to get the python bindings working on Windows. Since the function declarations are the only place where dllexport is applied, I ran into undefined functions. This is not the case on Ubuntu because all the function definitions also have export "C", so they were being exported despite the function declarations not matching.

stella.stamenova edited the summary of this revision. (Show Details)
stella.stamenova added a reviewer: rriddle.
rriddle accepted this revision.May 5 2022, 12:52 PM

This change LGTM, standardizing around the camelCase naming convention seems desirable here.

This revision is now accepted and ready to land.May 5 2022, 12:52 PM
This revision was automatically updated to reflect the committed changes.

Thanks for fixing this!