steakhal added a subscriber: NoQ. steakhal added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:589 + RetTypeLoc.getType().getCanonicalType().getAsString(); + if (CanonicalTypeStr == "id" || CanonicalTypeStr == "instancetype") { + return None; ---------------- You should avoid `std::string` comparisons. `llvm::StringRefs` are a better alternative. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:592-602 + // If we're inside of an NS_ASSUME, then the sourceRange will end before the + // asterisk. + const auto CanonicalTypeSize = CanonicalTypeStr.size(); + const bool IsInsideOfAssume = + NullabilityLoc == RetTypeLoc.getSourceRange().getBegin().getLocWithOffset( + CanonicalTypeSize - 1); + ---------------- Uh, this is really ugly. I don't believe this is the preferred way of detecting this. @NoQ WDYT? ================ Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:687-688 + + const bool ContextSupportsSyntaxSugar = + dyn_cast<ObjCMethodDecl>(D) != nullptr; + llvm::Optional<FixItHint> Hint = ---------------- ================ Comment at: clang/test/Analysis/nullability-fixits.mm:5-10 +// RUN: %clang_cc1 -analyze \ + +// RUN: -Wno-nonnull \ +// RUN: -analyzer-checker=core,nullability.NullableReturnedFromNonnull,nullability.NullReturnedFromNonnull \ +// RUN: -fdiagnostics-parseable-fixits %s 2>%t +// RUN: cat %t | FileCheck %s -check-prefix=CHECK-FIXIT-DISABLED-IMPLICIT ---------------- Why don't you use the `%clang_analyze_cc1 ...` form instead or even better the `%check_analyzer_fixit` tool-subst pattern? See the `clang/test/Analysis/dead-stores.c` as an example. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123352/new/ https://reviews.llvm.org/D123352 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits