[PATCH] Start fleshing out the Darwin driver

[Nick Kledzik]

On Aug 22, 2013, at 12:43 AM, Joe Ranieri <joe at alacatialabs.com> wrote:

> Here is a patch that adds a handful of options to the Darwin driver. There's only a few because I'd like to know if I'm on the right track or not. I've tried to model the implementation and style off of the various existing bits of drivers and linking contexts for each platform.
> 
> I have a few questions, I suspect most of which are for Nick:
> - What should the granularity of the tests in DarwinLdDriverTest.cpp be? For now I've put all of the new dylib flags into the "Dylib" test instead of each flag getting its own test. This seems to mirror WinLinkDriverTest.cpp's "Basic" test, though it also has tests for individual options.
The Basic test should just be a sanity check.  Each option should have it own test that which tries variations on the option.  For instance -mark_dead_strippable_dylib should be tried with and without that option when building a dylib (and verify the expected value in _ctxt.deadStrippableDylib(), and try adding it when not building a dylib to verify the error/warning.   For the options that take a parameter (e.g. -current_version 1.0), there should be a number of tests that try different legal an illegal values (e.g. -1.0, 1.2.3.4.5, foo, etc).

> - Are the doxygen in MachOLinkingContext too verbose? Should they just be basic setters/getter pairs with no comments, like PECOFFLinkingContext?
I’m good with more documentation :-)


> - How should the dylib compatibility and current version flags be stored? The minimum OS version is immediately validated outside of validateImpl and stored into a PackedVersion. There is no getter to access the exact value for that field, just the minOS helper.
The client of these version fields is the mach-o writer which needs the 32-bit value to place into the mach-o data structures.  Therefore, the value should be stored in the context as a 32-bit value (not as a string).   What this means is that the step that gets the -current_version arg needs to validate (can be parsed into packed 32-bit value) and report error if not (instead of doing the checking in validateImp).  The PackedVersion inner class should probably be removed and add utility methods  to the MachOLinkingContext class to convert uint32_t <-> StringRef.


> - What are the style guidelines for the tablegen files and the unit tests? The LDOptions.td file seem to disregard the 80 column idea completely.
I try to keep DarwinOption.td to 80 columns.

-Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130822/4de0cccb/attachment.html>