tom-anders added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:538
+ for (size_t I = 0; I < FD->getNumParams(); ++I) {
+ if (const auto *Param = FD->getParamDecl(I)) {
+ auto T = Param->getType();
----------------
sammccall wrote:
> I feel like you'd be better off using the FunctionProtoType and iterating
> over argument types, rather than the argument declarations on a particular
> declaration of the function.
>
> e.g. this code is legal in C:
> ```
> int x(); // i suspect this is the canonical decl
> int x(int); // but this one provides the type
> ```
> We don't have references in C of course!, but maybe similar issues lurking...
I'm not really sure how to get from the CallExpr to the FunctionProtoType, can
you give me a hint?
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:547
+ if (isa<DeclRefExpr>(Arg)) {
+ Location = Arg->getBeginLoc();
+ } else if (auto *M = dyn_cast<MemberExpr>(Arg)) {
----------------
sammccall wrote:
> 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!
np :D
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
