[PATCH] D139215: [ORC][test] Add initial MachOPlatformTest

Fri Dec 2 15:30:35 PST 2022

dblaikie added a comment.

In D139215#3967637 <https://reviews.llvm.org/D139215#3967637>, @keith wrote:

> 2 thoughts:
>
> 1. having any tests for this class might be nice for encouraging others to add tests when they change this type, since this would already be setup
> 2. there are other callers to this function so regardless of what ORC keeps doing, this makes sure this functionality continues to work as expected for the other callers
>
> but yea I don't feel strongly, so i can drop if you think that's the right move

I can only find the one caller right now - and we tend to test fairly high level (relatively speaking) in LLVM - usually testing at the tool/program level, rather than down at the lowest API levels. (& if were strong unit test/API test adherents, I guess we'd have to test this function's behavior, then test that the callers call this function (with stubs/mocks/that sort of thing) - but that's not really the way LLVM goes for the most part, so testing a bit higher level, but still testing all the different sections here seems good to me)

(I assume the dynamic linker just has this as a hardcoded list somewhere too, @lhames Or should there be some special section flag or the like that is meant to help identify these/make it possible to add new ones witohut having to update linkers/loaders/etc)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139215/new/

https://reviews.llvm.org/D139215