[PATCH] D21075: Correct invalid end location in diagnostics for some identifiers.

[Richard Smith via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Sema/SemaExpr.cpp:2061-2062
+  auto Builder = Diag(R.getNameLoc(), diagnostic) << Name;
+  if (Name.isIdentifier())
+    Builder << SourceRange(R.getNameLoc());
   return true;
----------------
I'm indifferent on this change: I don't see much point in adding a single-token range when we already point the caret at that token, but I don't see any harm either.


================
Comment at: lib/Sema/SemaType.cpp:1339
+        auto R = DS.getSourceRange();
+        if (R.getEnd().isInvalid())
+          R.setEnd(R.getBegin());
----------------
klimek wrote:
> erikjv wrote:
> > klimek wrote:
> > > Do you know in which cases we get source ranges that are half valid?
> > In Parser::ParseTypeofSpecifier the 'cast range' for a typeof(...) is parsed, but is found to be invalid. Then it explicitly sets the DeclSpec's end location to an invalid location.
> Ok, in this case, we'll need Richard to verify whether he intended half-valid source locations something that users need to deal with.
The code in `ParseTypeofSpecifier` does this:

```
  if (CastRange.getEnd().isInvalid())
    // FIXME: Not accurate, the range gets one token more than it should.
    DS.SetRangeEnd(Tok.getLocation());
  else
    DS.SetRangeEnd(CastRange.getEnd());
```

This should always set the end of the range to a valid location. Generally, I think we should avoid half-valid ranges rather than trying to cope with them downstream.


https://reviews.llvm.org/D21075