Implement custom symbols referencing a section for machO

[bjorn3]

cc bjorn3/rustc_codegen_cranelift#435

[m4b]

Seems weird to pass in a btree map; at least from the docstring wasn’t clear to me as a user how to construct this particular input; also, what are the invariants I should enforce, etc.

Note I didn’t read the implementation deeply, just try signature stuck out at me.

Could you give some motivation for why we should expose this in this manner ? :)

[bjorn3]

This was the easiest way :) Should I introduce a SymbolMap as wrapper?

Invariants

• The u64 is smaller than the length of data (no out of bound symbols)
• No symbol name (the String) is used more than once.
• symbols is empty for non section definitions.

[m4b]

I guess I thought the api would be like define_in_section(name: String, data: Vec<u8>, section: String)

Why does one define a symbol, it’s data, and then provide a set of strings (section name ?) -> u64s (offsets?)

A symbol is only ever in one section, so I’m confused why the api has a collection like thing as an argument.

[bjorn3]

I did it the other way around: A section decl has symbols referencing to certain offsets. symbols is a map from symbol to offset within section.

[m4b]

Ok I haven’t heard from anyone so I’m ok to merge this, but I still have a few questions.

So the symbols argument to define_with_symbols; the docs state it’s a map of custom symbols referencing a section; but where does the section come from? Is it the symbol we’re defining?

The references seem like they’re “floating” to me, relying on previous or post function calls to resolve them, is that correct ?

If yes, that worries me, because it’s essentially introduced a stateful aspect to the api; is there any way we can make the connection stronger (short of moving symbol references to interned ids (which I somewhat regret not having in the first place))

I guess the api just doesn’t seem totally clear to me; a doc example would help a lot I think

[bjorn3]

Is it the symbol we’re defining?

Yes

The references seem like they’re “floating” to me, relying on previous or post function calls to resolve them, is that correct ?

The symbols referencing the section are newly introduced when calling this method.

I guess the api just doesn’t seem totally clear to me; a doc example would help a lot I think

Will add.

[m4b]

Ok got it now; yea doc example would be really great, and maybe slightly more explanation in documentation, but example I think will be best solution to my confusion/questions.

Then I think we can merge. Ping me again aggressively if I forget :)

[bjorn3]

Added doc example.

[bjorn3]

Ping @m4b

[m4b]

@pchickey @sunfishcode going to merge this in less than 24 hours, so speak now or forever hold your peace :)

[pchickey]

Works for me!

[bjorn3]

Thanks!