Diff Detail
- Repository
- rG LLVM Github Monorepo
| Time | Test | |
|---|---|---|
| 60,050 ms | x64 debian > MLIR.Examples/standalone::test.toy Script:
--
: 'RUN: at line 1'; /usr/bin/cmake /var/lib/buildkite-agent/builds/llvm-project/mlir/examples/standalone -G "Ninja" -DCMAKE_CXX_COMPILER=/usr/bin/clang++ -DCMAKE_C_COMPILER=/usr/bin/clang -DLLVM_ENABLE_LIBCXX=OFF -DMLIR_DIR=/var/lib/buildkite-agent/builds/llvm-project/build/lib/cmake/mlir -DLLVM_USE_LINKER=lld -DPython3_EXECUTABLE="/usr/bin/python3.9"
|
Event Timeline
Per the discussion on https://discourse.llvm.org/t/i-accidentally-created-a-naming-inconsistency-in-the-ocaml-api/65715, I renamed the pointer_type_in_context function I previously added. Because this function takes an address space as an argument, I decided to rename it to qualified_pointer_type2, rather than pointer_type2, to match the existing qualified_pointer_type function, which also takes an address space as an argument. Then, I added another function, pointer_type2, which uses the opaque pointer API and does not take an address space as an argument, to correspond with the pointer_type function.
I also changed as to a in the documentation because as is an OCaml keyword and therefore not a valid variable name. (The occurrence of as in the documentation comment was colored like a keyword in my editor, so it stood out to me.)
Also, I chose not to use the word "opaque" in the docs because once typed pointers are phased out, *all* pointers will be opaque pointers.
According to https://llvm.org/docs/OpaquePointers.html#version-support, LLVM 16 will not support typed pointers. So, if this patch is not going into an LLVM 15 minor or patch release, could one totally remove the old pointer_type and qualified_pointer_type functions and give their names to pointer_type2 and qualified_pointer_type2? However, that involves rewriting the tests, which use the old functions, and I'm not an expert on what all the tests are supposed to check...
I now made https://reviews.llvm.org/D135524, which is an alternative to this patch. For context, see https://discourse.llvm.org/t/i-accidentally-created-a-naming-inconsistency-in-the-ocaml-api/65715. I intend for either one or the other to be merged depending on which my reviewers think is more appropriate.