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

[Douglas Gregor]

On Jun 22, 2009, at 10:18 AM, Ted Kremenek wrote:

> Hi Argiris,
>
> I was looking at this patch, and it wasn't clear to me how is this
> useful?  Looking at the definition of operator<= tells me little about
> its intended semantics or when it is intended to be used.  My concern
> is that some users may expect operator<= to have some notion of line
> and column comparisons, which certainly isn't the case.  Do we really
> need to use operator overloading?  If so, could you add a comment of
> where this is intended to be used?
>
> Thanks,
> Ted

It looks like SourceLocation already had a <, which is some kind of  
total ordering on SourceLocations that doesn't reflect their relative  
order within the translation unit. As Ted noted in an offline  
discussion about this, all source locations that come from macros will  
come later in this ordering than source locations that relate to a  
specific file/line/column. So while < is a total ordering, it's only  
really useful for using std::set or std::map, and conveys no semantic  
meaning.

I do think we need the ability to provide a total ordering on source  
locations that does have the semantic meaning of "source location x <  
source location y if x occurs before y in the translation unit". I  
suspect that this will have to be a function object that involves the  
SourceManager to deal with comparing source locations that point into  
macro instantiations, and it's not going to be as cheap as comparing  
the raw encodings.

Then, since operator< is unlikely to have a real semantic meaning, it  
probably makes sense to remove it in favor of a function object that  
can be used with std::set, std::map, etc.

	- Doug

> On Jun 20, 2009, at 1:08 AM, Argiris Kirtzidis wrote:
>
>> Author: akirtzidis
>> Date: Sat Jun 20 03:08:38 2009
>> New Revision: 73820
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=73820&view=rev
>> Log:
>> Add operator '<=' for comparing SourceLocations.
>>
>> Modified:
>>   cfe/trunk/include/clang/Basic/SourceLocation.h
>>
>> Modified: cfe/trunk/include/clang/Basic/SourceLocation.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceLocation.h?rev=73820&r1=73819&r2=73820&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> = 
>> =====================================================================
>> --- cfe/trunk/include/clang/Basic/SourceLocation.h (original)
>> +++ cfe/trunk/include/clang/Basic/SourceLocation.h Sat Jun 20
>> 03:08:38 2009
>> @@ -145,6 +145,10 @@
>>  return LHS.getRawEncoding() < RHS.getRawEncoding();
>> }
>>
>> +inline bool operator<=(const SourceLocation &LHS, const
>> SourceLocation &RHS) {
>> +  return LHS.getRawEncoding() <= RHS.getRawEncoding();
>> +}
>> +
>> /// SourceRange - a trival tuple used to represent a source range.
>> class SourceRange {
>>  SourceLocation B;
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits