nridge requested changes to this revision.
nridge added a comment.
This revision now requires changes to proceed.
Thanks, this seems like a useful extension of the `usedAsMutableReference`
modifier
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:541
+ return Base::TraverseConstructorInitializer(Init);
+ if (const auto Member = Init->getMember()) {
+ const auto MemberType = Member->getType();
----------------
Use `auto *Member` to make clang-tidy happy (`const auto *` is also fine)
Likewise in a few places below
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:543
+ const auto MemberType = Member->getType();
+ if (MemberType->isLValueReferenceType() &&
+ !MemberType->getPointeeType().isConstQualified()) {
----------------
Can we factor out [this
logic](https://searchfox.org/llvm/rev/740633ff08ff2d84d1f06088a12d9381b4c83c1b/clang-tools-extra/clangd/SemanticHighlighting.cpp#577-598)
into a helper function `highlightMutableReferenceArgument(QualType TargetType,
const Expr *Arg)`?
Then here we can call `highlightMutableReferenceArgument(MemberType,
Init->getInit())`
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:563
llvm::ArrayRef<const Expr *> Args = {E->getArgs(), E->getNumArgs()};
if (const auto callOp = dyn_cast<CXXOperatorCallExpr>(E)) {
switch (callOp->getOperator()) {
----------------
nits from previous patch -- I'm getting clang-tidy diagnostics on this line
about:
1. naming (should be `CallOp`)
2. it seems clang-tidy prefers writing this as `auto *CallOp = `
Could you fix these as part of this patch?
================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:752
+ $Field_readonly[[i2]]($Parameter[[i]]),
+ $Field[[i3]]($Parameter_usedAsMutableReference[[i]]) {}
+ int $Field_decl[[i1]];
----------------
Maybe we can add a line where the initializer expression is not a constructor
parameter, but e.g. a non-const static data member of another class.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128977/new/
https://reviews.llvm.org/D128977
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
