ilya-biryukov added a comment.
In D67901#1704639 <https://reviews.llvm.org/D67901#1704639>, @nridge wrote:
> I like how we went from using heuristics, to not using heuristics, to using
> heuristics again :)
>
> But admittedly, the new heuristics are more accurate because they're based on
> phase 1 lookup results rather than just syntax, so I'm happy with the outcome!
I also liked the irony of it :-)
Mostly LG, just NITs from my side. Happy to LGTM when they're addressed.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:105
+ for (NamedDecl *Decl : Decls) {
+ if (TemplateDecl *TD = dyn_cast<TemplateDecl>(Decl)) {
+ Decl = TD->getTemplatedDecl();
----------------
Could we do this in `kindForDecl` instead?
I suspect we want to be consistent in other cases this might potentially called
in.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:108
+ }
+ llvm::Optional<HighlightingKind> Kind = kindForDecl(Decl);
+ if (!Kind)
----------------
Maybe simplify the rest of the loop body to the following code?
```
auto Kind = ...;
if (!Kind || Result && Result != Kind)
return llvm::None;
Result = Kind;
```
If you want to have fewer assignments, we could also do:
```
if (!Result) Result = Kind;
```
at the end. But I'd just keep it a tad simpler instead.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67901/new/
https://reviews.llvm.org/D67901
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
