[PATCH] Complete Rewrite of CGRecordLayoutBuilder
yunzhong_gao at playstation.sony.com
Thu Feb 20 14:04:38 PST 2014
The rest of the patch looks good by me. You might want to check with John if he has any comments.
From: Reid Kleckner [rnk at google.com]
Sent: Wednesday, February 19, 2014 5:43 PM
To: reviews+D2795+public+7f6ebea2b933198a at llvm-reviews.chandlerc.com
Cc: John McCall; David Majnemer; Warren Hunt; cfe-commits at cs.uiuc.edu cfe; Gao, Yunzhong
Subject: Re: [PATCH] Complete Rewrite of CGRecordLayoutBuilder
On Wed, Feb 19, 2014 at 5:41 PM, Yunzhong Gao <Yunzhong_Gao at playstation.sony.com<mailto: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
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,
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.
More information about the cfe-commits