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

Reply via email to