JonasToth added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp:34-41
+static bool isArrayType(QualType QT) { return isa<ArrayType>(QT.getTypePtr());
}
+static bool isReferenceType(QualType QT) {
+ return isa<ReferenceType>(QT.getTypePtr());
+}
+static bool isPointerType(const Type *T) { return isa<PointerType>(T); }
+static bool isPointerType(QualType QT) {
+ return isPointerType(QT.getTypePtr());
----------------
aaron.ballman wrote:
> I don't see how these are improvements over checking `QT->isArrayType()` and
> friends within the caller.
True!
================
Comment at: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp:82
+ return (llvm::Twine(' ') + DeclSpec::getSpecifierName(Qualifier)).str();
+ return (llvm::Twine(DeclSpec::getSpecifierName(Qualifier)) + llvm::Twine('
'))
+ .str();
----------------
aaron.ballman wrote:
> Do you need both casts for `Twine`? I would imagine only the first is needed.
In case of `' '` it is necessary, in case of `" "` its not. I go with `" "`,
should not be more expensive or anything, right?
================
Comment at: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp:166
+
+ llvm_unreachable("All paths should have been handled");
+}
----------------
aaron.ballman wrote:
> I think this path is reachable -- it should be an `assert()` instead and
> return `None`.
When testing LLVM i saw some path triggering the assertions and unreachables.
I want to go the `None`-route for now and only emit warnings without fixit.
That will help to find false-positives unhandled cases better and is less
problematic.
Is it only objc-code that could trigger this?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54395/new/
https://reviews.llvm.org/D54395
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
