[cfe-commits] r44942 - /cfe/trunk/include/clang/Basic/SourceLocation.h

[Chris Lattner]

> URL: http://llvm.org/viewvc/llvm-project?rev=44942&view=rev
> Log:
> Added class FullContextSourceLocation: a tuple class that
> contains both a SourceLocation and its associated
> SourceManager. This class is useful for argument passing to
> functions that expect both objects.

Hey Ted,


> +/// FullContextSourceLocation - A tuple containing both a  
> SourceLocation
> +///  and its associated SourceManager.  Useful for argument  
> passing to functions
> +///  that expect both objects.
> +class FullContextSourceLocation {
> +  SourceLocation Loc;
> +  SourceManager* SrcMgr;
> +public:

Ok.

> +  explicit FullContextSourceLocation(SourceLocation loc)
> +    : Loc(loc), SrcMgr(NULL) {}

I don't think we want to support fullsourceloc's with a null srcmgr  
unless the location is invalid.  It seems best to support two ctors:  
a default ctor that makes an invalid sloc, and a version that takes  
both.

> +
> +  explicit FullContextSourceLocation(SourceLocation loc,  
> SourceManager& smgr)
> +    : Loc(loc), SrcMgr(&smgr) {}

I think all clients that decode the sloc only need read-only access  
to the source manager.  I'd suggest making all references to it const.

> +  SourceLocation getSourceLocation() const { return Loc; }
> +  operator SourceLocation() const { return Loc; }

Ick.  Please name this getLoc(), and remove the implicit conversion :)

> +
> +  SourceManager& getSourceManager() {

Please drop this one (no need for a non-const one).

> +  const SourceManager& getSourceManager() const {

How about getManager() ?

-Chris