[PATCH] Implementation of #pragma (data|code|const|bss)_seg

[Aaron Ballman]

On Mon, Apr 7, 2014 at 9:25 PM, Warren Hunt <whunt at google.com> wrote:
>   Addresses Aaron's comments.  Also rebased and incorporates init_seg using the unified MS pragma handling machinery.
>   Specific comments:
>   * added documentation for the __declspec(allocate) attribute
>   * warnings were added and expanded upon, but could not be fused
>   * XXXXSeg renamed to Segment
>   * Fixed style issues around &and *
>   * SectionInfo will be read during codegen (we can make it private and add an interface if we really need to, noone's going to access it for anything they don't need...)
>   * Added a significant number of parser tests, found a bug and fixed that
>   * Added a custom diagnostic for shared, nopage, nocache, discard and remove
>   * Fixed things that would break MSVC2012
>   * Other random small fixes requested.

Changes all look good. The only issue I caught this time was some
gratuitous commas in your enum definitions. If you could remove those
from PragmaMsStackAction, PragmaMSKind, and PragmaSectionFlag, that'd
be great.

>   Specific Replies:
>   * We're using eof instead of eod for our pragmas due to the way that certain parser helpers work.  Things like parsestringliteral can walk through an eod but not an eof.  Richard Smith suggested this.

Ah, makes sense!

>   * After talking to a couple people here about [&] for the lambdas (I'm new to lambdas and am looking for guidance) there was some consensus that these lambdas are both so short and used so locally that being more explicit about what exactly they capture is not particularly more clear and adds code.

I don't fully agree with that rationale, as it makes it harder to
refactor code later, as additions of locals or parameters suddenly
start adding cost in code that may not require updating. However, I
think that's more of a broad issue for the style guidelines, and not
something I'm super concerned about for your particular usages.

>   * The Add followed by Remove process (rather than making the add conditional based on it not being immediately removed) significantly simplified the implantation logic by allowing Unify to always look at the attribute.  Given that this particular piece of code is colder than cold (someone is mis-using an extremely rare and specific pragma) I opted for code size/simplicity over performance.

Makes sense to me.

So aside from my minor nit about enums, LGTM! Thanks!

~Aaron