[cfe-commits] r74197 - in /cfe/trunk: include/clang/Frontend/Utils.h lib/Frontend/CMakeLists.txt lib/Frontend/ResolveLocation.cpp

[Argyrios Kyrtzidis]

On Jun 25, 2009, at 4:10 PM, Douglas Gregor wrote:

>
> On Jun 25, 2009, at 4:05 PM, Argyrios Kyrtzidis wrote:
>
>> On Jun 25, 2009, at 3:36 PM, Douglas Gregor wrote:
>>
>>>
>>> Okay, so the scheme here is: check whether the source location we're
>>> looking for is within the range of the current node. If so,  
>>> recurse to
>>> find which sub-node it is in. I like that, although the CheckRange/
>>> FoundIt/Finished trio feels more complicated that it needs to be,
>>> because there's a lot of state encoded in Dcl/Stm/PassedLoc. Why not
>>> just have your Visit functions return std::pair<Decl*, Stmt*> (or
>>> something like it?).
>>
>> I initially had them returning a class with Decl, Stmt, bool  
>> (PassedLoc), but I eventually moved that state into the LocResolver  
>> class to avoid constantly passing the (mostly same) structure  
>> amongst the visitor methods.
>
> Was it unwieldy, or were you worried about performance?

Performance. (yeah, I was enchanted by the "premature optimization"  
mistress)

>
>> I don't have a strong opinion though, let me know if you think  
>> returning the state from the visitor methods is better.
>
>
> It seems like it would be cleaner (more functional) to return the  
> state from the visitor methods, but if you've tried it and didn't  
> like it, that's fine, too.

I'll do that.

Thanks for reviewing!

-Argiris