nridge added inline comments.

================
Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:264
+        // FIXME: This one does not work yet.
+        A<T>::static_member($par3[[t]]);
       }
----------------
sammccall wrote:
> nridge wrote:
> > This is an interesting case. Clang builds a `CXXDependentScopeMemberExpr` 
> > for this callee, but `HeuristicResolver` [currently 
> > assumes](https://searchfox.org/llvm/rev/92880ab7a2b2145f0605f367cd6d53d6892903c3/clang-tools-extra/clangd/HeuristicResolver.cpp#108)
> >  that such expressions are only built for non-static member accesses 
> > (since, for static member accesses, clang usually builds a 
> > `DependentScopeDeclRefExpr` instead).
> > 
> > The `CXXDependentScopeMemberExpr` is created 
> > [here](https://searchfox.org/llvm/rev/92880ab7a2b2145f0605f367cd6d53d6892903c3/clang/lib/Sema/SemaTemplate.cpp#757),
> >  and I note the dependence on whether the //enclosing context// is an 
> > [instance 
> > method](https://searchfox.org/llvm/rev/92880ab7a2b2145f0605f367cd6d53d6892903c3/clang/lib/Sema/SemaTemplate.cpp#750).
> >  I guess it must think that, after instantiation, `A<T>` could turn out to 
> > be a base class, and thus this could be a "non-static member access with 
> > qualifier".
> > 
> > I don't see anything obvious on `CXXDependentScopeMemberExpr` that would 
> > let us tell apart "definitely a non-static member" from "maybe static, 
> > maybe non-static", so I guess the appropriate solution is to drop the 
> > `NonStaticFilter` 
> > [here](https://searchfox.org/llvm/rev/92880ab7a2b2145f0605f367cd6d53d6892903c3/clang-tools-extra/clangd/HeuristicResolver.cpp#108)
> >  altogether?
> > I guess it must think that, after instantiation, A<T> could turn out to be 
> > a base class, and thus this could be a "non-static member access with 
> > qualifier".
> 
> Argh, C++ is so tricky :-( That sounds plausible to me.
> 
> > drop the NonStaticFilter here altogether
> 
> Yeah. The other thing is that `some_instance.some_static_member` is perfectly 
> legal I think? So the NonStaticFilter is probably not correct anyway.
This is sufficiently non-trivial to fix (requires more than just removing the 
filter) that I'll leave it for a separate patch / review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100742

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

Reply via email to