[cfe-commits] [PATCH 1/2] [clang.py] Add TranslationUnit.get_{file, source_location, source_range}

[Manuel Klimek]

High-level remark:
The idea of having multiple different parameters of which only one must
ever be set seems strange to me. Why not instead have multiple methods with
different names? I'd also prefer having 1 parameter and finding out what
methods it provides.

Cheers,
/Manuel

On Sat, Jun 30, 2012 at 4:15 AM, Gregory Szorc <gregory.szorc at gmail.com>wrote:

> Updated patch.
>
> On Fri, Jun 29, 2012 at 8:47 AM, Gregory Szorc <gregory.szorc at gmail.com>
> wrote:
> > Having thought about this in my sleep, I may want to rescind this
> > review request and refactor things a little.
> >
> > 1) I may change "range" and "source_range" names to "extent" since
> > that is what is used elsewhere.
> > 2) I may combine the bounds to obtain ranges/extents from two
> > arguments to 2-tuples.
> > 3) I may remove the "_source" from get_source_location and
> > get_source_extent. I don't think that's any less clear.
> >
> > This will of course invalidate the token API patch that followed,
> > albeit trivially.
> >
> > On Fri, Jun 29, 2012 at 12:13 AM, Gregory Szorc <gregory.szorc at gmail.com>
> wrote:
> >> These are just convenience APIs to make obtaining File,
> >> SourceLocation, and SourceRange instances easier.
> >>
> >> Old way:
> >>
> >> f = File.from_name(tu, 'foo.c')
> >> location = SourceLocation.from_offset(tu, f, 10)
> >>
> >> New way:
> >>
> >> location = tu.get_source_location('foo.c', offset=10)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120702/35f5dc1b/attachment.html>