sammccall added inline comments.

================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:547
+            if (isa<DeclRefExpr>(Arg)) {
+              Location = Arg->getBeginLoc();
+            } else if (auto *M = dyn_cast<MemberExpr>(Arg)) {
----------------
tom-anders wrote:
> sammccall wrote:
> > nridge wrote:
> > > For a qualified name (e.g. `A::B`), I think this is going to return the 
> > > beginning of the qualifier, whereas we only want to highlight the last 
> > > name (otherwise there won't be a matching token from the first pass).
> > > 
> > > So I think we want `getLocation()` instead.
> > > 
> > > (Also makes a good test case.)
> > And getLocation() will do the right thing for DeclRefExpr, MemberExpr, and 
> > others, so this can just be `isa<DeclRefExpr, MemberExpr>` with no need for 
> > dyn_cast.
> I'm not sure which getLocation() you're talking about here. There's 
> DeclRefExpr::getLocation(), but neither Expr::getLocation() nor 
> MemberExpr::getLocation(). Am I missing something?
No, I think I'm just going mad (I was thinking of Decl::getLocation I guess).
Never mind and sorry!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108320/new/

https://reviews.llvm.org/D108320

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

Reply via email to