zhengkai added a comment.

In http://reviews.llvm.org/D12379#245892, @rtrieu wrote:

> In http://reviews.llvm.org/D12379#245868, @zhengkai wrote:
>
> > In http://reviews.llvm.org/D12379#245861, @rtrieu wrote:
> >
> > > In http://reviews.llvm.org/D12379#245850, @zhengkai wrote:
> > >
> > > > In http://reviews.llvm.org/D12379#245849, @rtrieu wrote:
> > > >
> > > > > Why are you leaking the raw encoding of locations from the 
> > > > > SourceManager?  This is an internal implementation detail and should 
> > > > > not be relied on externally.  SourceLocation is the typical way to 
> > > > > use locations.
> > > >
> > > >
> > > > Because I want to check if all the ranges fit in the same argument of 
> > > > one macro invocation, they can be different Source Locations. In 
> > > > ExpansionInfo, these locations have the same ExpansionLocStart.
> > >
> > >
> > > If you call ExpansionInfo::getExpansionLocStart(), then the 
> > > SourceLocation will be the same if the ExpansionLocStart is the same.
> >
> >
> > But I still need to change the isMacroArg function to get the result of 
> > getExpansionLocStart(), and if I want to compare two SourceLocation, I 
> > still need to check the raw encoding. I don't know if it is needed not to 
> > leak the raw encoding.
>
>
> I kind of see what you are trying to do here, and we should fix it a 
> different way.
>
> 1. Get rid of changes to ExpansionInfo
> 2. Change the function SourceManager::isMacroArgExpansion to take two 
> arguments instead of one.  The new argument should be a SourceLocation 
> pointer named MacroBegin.
> 3. Change the last line of SourceManager::isMacroArgExpansion from "return 
> Expansion.isMacroArgExpansion();" to "if (!Expansion.isMacroArgExpansion()) 
> return false;"  After that, add the code to get the macro begin location.
> 4. Update DiagnosticRenderer to use SourceLocation instead of raw encodings.


Have changed the implementation.


http://reviews.llvm.org/D12379



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to