aaron.ballman added inline comments.
================ Comment at: clang-tidy/utils/FixItHintUtils.cpp:35 +static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); } +static bool isArrayType(QualType QT) { return isa<ArrayType>(QT.getTypePtr()); } +static bool isReferenceType(QualType QT) { ---------------- This file is replicating a bunch of functionality that exists on the `Type*` already. Why do this rather than have the caller do `QT->isArrayType()` and skip this function entirely? (Same exists elsewhere here.) ================ Comment at: clang-tidy/utils/FixItHintUtils.cpp:109 + const ASTContext &Context) { + // The pointer itself shall be marked as `const`. This is always right + // of the '*' or in front of the identifier. ---------------- always right -> always to the right ================ Comment at: clang-tidy/utils/FixItHintUtils.h:47 +/// \c SourceLocation it is not returned. +Optional<FixItHint> changeVarDeclToConst(const VarDecl &Var, + ConstTarget CT = ConstTarget::Pointee, ---------------- I feel like this could be trivially generalized to any type qualifier, not just `const`. How would you feel about renaming this to: `addQualifierToVarDecl()` and adding a parameter of type `DeclSpec::TQ` to specify which qualifier to add? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54395/new/ https://reviews.llvm.org/D54395 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits