From: Chandler Carruth [mailto:[email protected]]
Sent: Thursday, October 04, 2012 5:00 PM
To: Villmow, Micah
Cc: Kim Gräsman; Evan Cheng; [email protected] LLVM; Nadav Rotem; 
[email protected] cfe
Subject: Re: [cfe-commits] [llvm-commits] [Patch] Move TargetData from Target 
to Support/VMCore

On Thu, Oct 4, 2012 at 4:48 PM, Villmow, Micah 
<[email protected]<mailto:[email protected]>> wrote:
Here is my first run at implementing TargetData with DataLayout.

This should allow all the clients to start switching over to datalayout without 
any functionality breaking.
There are comments in TargetData that may need updating.
[Villmow, Micah] Ok, will do.

You've made the destructor virtual. I don't think you should need this. The 
code responsible for creating and destroying these will need to be the last 
migrated (which is fine) and that code should use TargetData consistently until 
every client is updated to use DataLayout. Then it can switch, and the virtual 
destructor won't matter.
[Villmow, Micah] Ok, I was only making it this way temporarily.

Some comments:

+ DataLayout(*reinterpret_cast<const DataLayout*>(&TD))

No, you should just call DataLayout(TD). Derived-to-base conversion will handle 
the rest.

With those changes, if everything builds and tests pass, LGTM.
[Villmow, Micah] Will have to wait till tomorrow to get the testing, having 
issues with windows testing now and this isn't a non-functional change.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to