[PATCH] Complete Rewrite of CGRecordLayoutBuilder

Reid Kleckner rnk at google.com
Wed Feb 19 17:43:28 PST 2014


On Wed, Feb 19, 2014 at 5:41 PM, Yunzhong Gao <
Yunzhong_Gao at playstation.sony.com> wrote:

>
>   A coding style question:
>
>   It seems that CGRecordLowering is exposing a lot of its member fields
> and functions. I wonder whether it makes sense to make some of these member
> fields private. For example, it seems
>     std::vector<MemberInfo> Members
>   does not need to be referenced by outside classes, and could be made
> private (and hence the definition of MemberInfo). I did not check all the
> "Input Memoization fields", but I suspect some of them do not need to be
> exposed either. And some of the member functions as well, such as,
>     void lowerUnion();
>   and some other functions used by lower(), probably can be hidden from
> outside classes as well.
>
>   I think this is minor point since CGRecordLowering is only defined and
> used in this file, and I am not entirely sure about LLVM's coding style on
> access control. What do the other reviewers think?
>

Adding private: around everything except the entry points is probably a
good idea, but I don't think it matters too much.  All that stuff is TU
local anyway.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140219/f923aa8c/attachment.html>


More information about the cfe-commits mailing list