[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

[Nathan Ridge via Phabricator via cfe-commits]

nridge marked 2 inline comments as done.
nridge added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:181
+      addToken(E->getMemberLoc(), E->getQualifier()
+                                      ? HighlightingKind::StaticField
+                                      : HighlightingKind::Field);
----------------
hokein wrote:
> ilya-biryukov wrote:
> > nridge wrote:
> > > hokein wrote:
> > > > This could be member functions, a case is like
> > > > 
> > > > ```
> > > > template<class T>
> > > > class Foo {
> > > > public:
> > > >   void foo() {
> > > >     this->foo();
> > > >   }
> > > > };
> > > > ```
> > > Thanks for the example.
> > > 
> > > Do you have a suggestion for how to discriminate this case? To me, it would seem logical to do it based on syntax (highlight as a member function if the expression forms the function name of a function call expression). That would require navigating from the expression to its parent node. Is there a way to do that?
> > There is no way to do this in C++.
> > Even if the name is followed by a pair of parenthese, this could either be a field with overloaded `operator()` (e.g. a `std::function<void()> field`) or a function with the same name.
> > 
> > It's much better to pick a separate highlighting kind for dependent names, this follows the actual semantics of C++.
> +1, I think we should just highlight them as a dependent type.
Of course, any attempt to disambiguate between a member function and a field would be heuristic only. I figured that would be better than nothing. But if you prefer using a separate highlighting for dependent names that resolve to a function or a variable, we could do that.

(Hokein, I assume you don't actually mean using the dependent *type* highlighting. Using a type highlighting for something we know is not a type, but rather a function or variable, would be rather confusing.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67901