[cfe-commits] r73823 - in /cfe/trunk: include/clang/Basic/SourceManager.h lib/Basic/SourceManager.cpp

[Argyrios Kyrtzidis]

On Jun 22, 2009, at 1:50 PM, Douglas Gregor wrote:

>
> On Jun 22, 2009, at 1:46 PM, Argyrios Kyrtzidis wrote:
>
>>
>> On Jun 22, 2009, at 10:16 AM, Douglas Gregor wrote:
>>
>>>
>>> On Jun 20, 2009, at 1:09 AM, Argiris Kirtzidis wrote:
>>>
>>>> Author: akirtzidis
>>>> Date: Sat Jun 20 03:09:57 2009
>>>> New Revision: 73823
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=73823&view=rev
>>>> Log:
>>>> Introduce SourceManager::getLocation() to get a source location out
>>>> of a "file:line:column" triplet.
>>>>
>>>
>>> Looks good. There is similar code in tools/clang-cc/clang-cc.cpp, in
>>> the command line option parsing for a ParsedSourceLocation. Can we
>>> combine the two?
>>
>> I don't think so, the SourceManager::getLocation only works if the  
>> file has already been parsed, while the command line locations are  
>> parsed before-hand.
>
> We could, at least, factor the parsing logic into a single routine  
> to extract file/line/column, right?

Yep, I intend to do that soon.

>
>> BTW, a bit off-topic but the FixIt tests are not "entirely correct"  
>> at testing the rewriter, i.e the "test/FixIt/fixit-at.c" has this:
>>
>> // RUN: clang-cc -fsyntax-only -fixit-at=fixit-at.c:3:1 %s -o - |  
>> clang-cc -verify -x c -
>>
>> if the rewriter does nothing, the second clang-cc  gets an empty  
>> file and the test will succeed.
>
>
> Yes, that's a good point. We should do something we can check, e.g.,  
> both compile the result and grep for certain substrings of the input  
> file.

Yes, one grep for each rewrite I think.

-Argiris