[PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:3403-3427
@@ -3402,5 +3402,6 @@
   // recovery, but emit a diagnostic and don't store the results.
-  SourceRange NoexceptRange;
+  SourceRange NoexceptRange(Tok.getLocation(),
+                            Tok.getEndLoc().getLocWithOffset(-1));
   ExceptionSpecificationType NoexceptType = EST_None;
 
   SourceLocation KeywordLoc = ConsumeToken();

@@ -3423,6 +3424,5 @@
   } else {
     // There is no argument.
     NoexceptType = EST_BasicNoexcept;
-    NoexceptRange = SourceRange(KeywordLoc, KeywordLoc);
   }
 
----------------
rsmith wrote:
> This change seems strange:
> 
>  1) We should be using normal token-based ranges here; the `getEndLoc().getLocWithOffset(-1)` doesn't make sense.
>  2) This specifies a range for the error case, where we produce an exception specification of `EST_None`. We should not have a `NoexceptRange` if we're recovering as if there were no `noexcept`.
> 
> Can you revert the changes to this file?
I agree that this code is strange, what I found was that anything other than getLocWithOffset would produce the wrong range (the resulting range would be off-by-one). I found other code that does this, but it may be equally incorrect if there's a better way to get a range for a token.

As for #2, I don't think that's actually the case. This is the code path that parses a noexcept specifier with no arguments. (Result starts as EST_None, we aren't delayed, there is no dynamic exception spec, the next token is noexcept, there is no l_paren, and so we set SpecificationRange and Result before returning). Without tracking the source range, the following test case fails because the token range has no length:
``` 
  Verifier.expectRange(1, 10, 1, 17);
  EXPECT_TRUE(Verifier.match("void f() noexcept;\n", loc(functionType()),
                             Language::Lang_CXX11));
```
Is there a better way for me to get the full source range of the token? I would have assumed that getEndLoc() would work, but it appears to give an inconsistent result with other ranges that we track. e.g., noexcept(false) the caret points at ), throw(...) the caret points at ), but without the -1 offset above for void f() noexcept; the caret points at ; instead of t.


http://reviews.llvm.org/D20428