[cfe-commits] r91683 - /cfe/trunk/include/clang/AST/CharUnits.h

[Ken Dyck]

On Tuesday, December 22, 2009 11:52 AM, Daniel Dunbar wrote:
>
> On Sat, Dec 19, 2009 at 11:14 AM, Daniel Dunbar 
> <daniel at zuster.org> wrote:
> > On Fri, Dec 18, 2009 at 9:56 AM, Chris Lattner 
> <clattner at apple.com> wrote:
> >>
> >> On Dec 18, 2009, at 5:39 AM, Ken Dyck wrote:
> >>
> >>> Author: kjdyck
> >>> Date: Fri Dec 18 07:39:46 2009
> >>> New Revision: 91683
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=91683&view=rev
> >>> Log:
> >>> Initial implementation of CharUnits, an opaque value class for 
> >>> representing sizes, offsets, and alignments in character units.
> >>
> >> This looks nice to me Ken, some minor comments:
> >>
> >>> +++ cfe/trunk/include/clang/AST/CharUnits.h Fri Dec 18 
> 07:39:46 2009
> >>> +#ifndef LLVM_CLANG_AST_CHARUNITS_H
> >>> +#define LLVM_CLANG_AST_CHARUNITS_H
> >>> +
> >>> +#include "llvm/ADT/StringExtras.h"
> >>
> >> Please move the toString() method out of line, to avoid 
> this #include.
> >
> > Actually, can you just remove the toString method()? Clients can do 
> > this themselves, its not a common operation, and I would like the 
> > #include <string> removed as well.
> >
> >  - Daniel
> >
> Ping?

Sorry. I've been on vacation and haven't had much time to work on this. 

Sure. I'll remove toString().

-Ken