This is an archive of the discontinued LLVM Phabricator instance.

[ORC][test] Add initial MachOPlatformTest
AbandonedPublic

Authored by keith on Dec 2 2022, 10:56 AM.

Details

Reviewers
lhames
dblaikie
Summary

Diff Detail

Event Timeline

keith created this revision.Dec 2 2022, 10:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2022, 10:56 AM
Herald added a subscriber: StephenFan. · View Herald Transcript
keith requested review of this revision.Dec 2 2022, 10:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2022, 10:56 AM

I'd probably skip this really low level/trivial test, and test just a bit higher level, as you've done in D139223

keith added a comment.Dec 2 2022, 1:42 PM

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

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)

keith abandoned this revision.Dec 2 2022, 4:28 PM