[PATCH] D12379: Fix the bugs in the mapDiagnosticRanges (Still in progress)

Zhengkai Wu via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 15 13:32:42 PDT 2015


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





More information about the cfe-commits mailing list