Re: [PATCH] D20134: [libclang] Fixed bug where ranges in spelling locations (in macro expansions) would get mangled

[Cameron via cfe-commits]

cameron314 added a comment.

You're right, this breaks that test. The corner case that it exercises is 
arguably less important than breaking all spelling ranges, though. But I'm 
going to see if I can fix both with a little wizardry (ideally there wouldn't 
be open ranges in the first place, since clang doesn't really support them, but 
failing that we can distinguish between an inclusive and exclusive source 
location internally and that might be enough).


http://reviews.llvm.org/D20134



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

Re: [PATCH] D20134: [libclang] Fixed bug where ranges in spelling locations (in macro expansions) would get mangled

[Richard Smith via cfe-commits]

rsmith added a comment.

I agree that this looks wrong.

Does test/Index/cindex-on-invalid.m still pass with this change? (See 
http://reviews.llvm.org/rL129872 in which it was added.) It looks like the 
problem is that we lose the information about whether we're supposed to be 
pointing to the start or the end of the token when we later map to the 
expansion location, which means that libclang's attempt to pretend that source 
ranges are half-open intervals rather than closed intervals breaks.


http://reviews.llvm.org/D20134



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

[PATCH] D20134: [libclang] Fixed bug where ranges in spelling locations (in macro expansions) would get mangled

[Cameron via cfe-commits]

cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.

The end location of the range would be changed into its expansion location, but 
the beginning of the range would stay as a spelling location. This would lead 
to very weird ranges spanning large parts of a file (starting in a macro until 
its expansion).

This bug was not previously caught because there was no way to actually obtain 
a spelling location via libclang (see my patch at D20125 which adds support for 
this), so later obtaining the start location of a range would result in an 
expansion location anyway, and wind up matching the expansion location that was 
stored in the end of the range.

http://reviews.llvm.org/D20134

Files:
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -142,8 +142,6 @@
   // We want the last character in this location, so we will adjust the
   // location accordingly.
   SourceLocation EndLoc = R.getEnd();
-  if (EndLoc.isValid() && EndLoc.isMacroID() && 
!SM.isMacroArgExpansion(EndLoc))
-EndLoc = SM.getExpansionRange(EndLoc).second;
   if (R.isTokenRange() && EndLoc.isValid()) {
 unsigned Length = Lexer::MeasureTokenLength(SM.getSpellingLoc(EndLoc),
 SM, LangOpts);


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -142,8 +142,6 @@
   // We want the last character in this location, so we will adjust the
   // location accordingly.
   SourceLocation EndLoc = R.getEnd();
-  if (EndLoc.isValid() && EndLoc.isMacroID() && !SM.isMacroArgExpansion(EndLoc))
-EndLoc = SM.getExpansionRange(EndLoc).second;
   if (R.isTokenRange() && EndLoc.isValid()) {
 unsigned Length = Lexer::MeasureTokenLength(SM.getSpellingLoc(EndLoc),
 SM, LangOpts);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits