[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