Page MenuHomePhabricator

move UseDwarfRangesBaseAddressSpecifier to front end flag
Needs ReviewPublic

Authored by mlekena on Dec 18 2017, 11:27 AM.



Move UseDwarfRangesBaseAddressSpecifier handling to the MC options command line argument.
This is a change to make the flag more accessible without needing to pass in arguments directly to the
LLVM backend.

Following this change, what would be a better name for this flag given that the
current name is rather verbose? Are there any opinions around this?

Event Timeline

mlekena created this revision.Dec 18 2017, 11:27 AM
aprantl added inline comments.Dec 18 2017, 11:50 AM

The LLVM coding guidelines want us to never comment out code; just remove it instead.

mlekena updated this revision to Diff 135257.Feb 21 2018, 7:45 AM
  1. Updating D41364: move UseDwarfRangesBaseAddressSpecifier to front end flag #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

Remove commented code as recommended to adhere to developers guide

dblaikie edited the summary of this revision. (Show Details)Feb 21 2018, 8:16 AM

As for a name (& might be worth using it here for the MCOption too) - maybe just "DwarfRangesBase" (Not sure if a "Use" prefix is helpful/necessary?), with the intent of the flag being -fdwarf-ranges-base

Open to hearing Adrian & others if they've got any other ideas (:

echristo added inline comments.Feb 21 2018, 2:43 PM

Might want to put this bool up near the other bools.


Typically we don't comment things out. :)

probinson added inline comments.Feb 21 2018, 3:04 PM

It looks like all this did was reorder some things. Is llc command processing really that sensitive to order?