nridge accepted this revision.
nridge added a comment.
This revision is now accepted and ready to land.
Looks good to me!
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:379
+ // ExternalLinkage threshold could be tweaked, e.g. module-visible as global.
+ // Avoid caching linkage if it may change after enclosing code completion.
+ if (hasUnstableLinkage(D) || D->getLinkageInternal() < ExternalLinkage)
----------------
code completion? I'm a bit lost :)
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:466
case TemplateArgument::TemplateExpansion:
+ // FIXME: I don't understand why this is DependentType.
H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
----------------
The testcase which relies on this is this one:
```
// Dependent template name
R"cpp(
template <template <typename> class> struct $Class[[A]] {};
template <typename $TemplateParameter[[T]]>
using $Typedef[[W]] = $Class[[A]]<
$TemplateParameter[[T]]::template $DependentType[[Waldo]]
>;
```
However, it does appear that we get into here even for non-dependent template
template arguments (but then also get a non-dependent highlighting kind via
`findExplicitReferences()`, and end up discarding the `DependentType` via
`resolveConflict()`).
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:785
+ case HighlightingModifier::FunctionScope:
+ return "functionScope";
+ case HighlightingModifier::ClassScope:
----------------
Maybe add a `// nonstandard` comment for these as well
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:82
+ FileScope,
+ GlobalScope,
+
----------------
Would it make sense to call this `NamespaceScope` instead?
I understand it's "global" in the sense that it's visible to other translation
units, but it's not "global" in the sense that it's not necessarily in the
global namespace.
(On the other hand, I can see how `FileScope` symbols are also "namespace
scope", so... could go either way.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95701/new/
https://reviews.llvm.org/D95701
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
