nridge added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:136
+// Whether D is const in a loose sense (should it be highlighted as such?)
+bool isConst(const Decl *D) {
+ if (llvm::isa<EnumConstantDecl>(D) || llvm::isa<NonTypeTemplateParmDecl>(D))
----------------
nridge wrote:
> sammccall wrote:
> > nridge wrote:
> > > Do you think in the future it might make sense to have the `readonly`
> > > modifier reflect, at least for variables, whether //the particular
> > > reference// to the variable is a readonly reference (e.g. binding the
> > > variable to a `const X&` vs. an `X&`)?
> > Do I understand correctly?
> >
> > ```
> > std::vector<int> X; // X is not readonly
> > X.push_back(42); // X is not readonly
> > X.size(); // X is readonly
> > ```
> >
> > Distinguishing potentially-mutating from non-mutating uses seems really
> > useful to me.
> > My only question is whether mapping this concept onto `readonly` is the
> > right thing to do:
> > - there are really three sets: const access, mutable access, and
> > non-access (e.g. declaration, or on RHS of `using a=b`). And I think maybe
> > the most useful thing to distinguish is mutable access vs everything else,
> > which is awkward with an attribute called "readonly".
> > - this also seems likely to diverge from how other languages use the
> > attribute (most don't have this concept)
> > - on the other hand, standard attribute names will be better supported by
> > clients
> >
> > This also might be somewhat complicated to implement :-)
> > I'd like to leave in this simple decl-based thing as a placeholder, and
> > either we can replace it and add an additional "mutation" attribute later
> > (once we work out how to implement it!) I've left a comment about this...
> >
> > (Related: a while ago Google's style guide dropped its requirement to use
> > pointers rather than references for mutable function params. Having `&x` at
> > the callsite rather than just `x` is a useful hint, but obviously diverging
> > from common practice has a cost. We discussed how we could use semantic
> > highlighting to highlight where a param was being passed by mutable
> > reference, though didn't have client-side support for it yet)
> > Do I understand correctly?
> >
> > ```
> > std::vector<int> X; // X is not readonly
> > X.push_back(42); // X is not readonly
> > X.size(); // X is readonly
> > ```
>
> Yup, that's what I had in mind.
>
> > Distinguishing potentially-mutating from non-mutating uses seems really
> > useful to me.
> > My only question is whether mapping this concept onto `readonly` is the
> > right thing to do:
> > - there are really three sets: const access, mutable access, and
> > non-access (e.g. declaration, or on RHS of `using a=b`). And I think maybe
> > the most useful thing to distinguish is mutable access vs everything else,
> > which is awkward with an attribute called "readonly".
> > - this also seems likely to diverge from how other languages use the
> > attribute (most don't have this concept)
> > - on the other hand, standard attribute names will be better supported by
> > clients
> >
> > This also might be somewhat complicated to implement :-)
> > I'd like to leave in this simple decl-based thing as a placeholder, and
> > either we can replace it and add an additional "mutation" attribute later
> > (once we work out how to implement it!) I've left a comment about this...
>
> Sounds good!
>
> > (Related: a while ago Google's style guide dropped its requirement to use
> > pointers rather than references for mutable function params. Having `&x` at
> > the callsite rather than just `x` is a useful hint, but obviously diverging
> > from common practice has a cost. We discussed how we could use semantic
> > highlighting to highlight where a param was being passed by mutable
> > reference, though didn't have client-side support for it yet)
>
> Funny you mention this, because I implemented this exact highlighting in
> Eclipse a few years ago:
>
> https://wiki.eclipse.org/CDT/User/NewIn92#Syntax_coloring_for_variables_passed_by_non-const_reference
>
> (and it's my main motivation for the suggestion I've made here).
> I implemented this exact highlighting in Eclipse a few years ago
I will readily admit that classifying references as mutable or not can get
tricky. For example:
```
int* array = new int[10];
array[0] = 42;
```
Is the reference to `array` on the second line mutable? Does the answer change
if the declaration is changed to `int array[10];`?
Fun questions to figure out later :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77811/new/
https://reviews.llvm.org/D77811
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
