[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145
+
+    diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null")
+        << FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),
----------------
aaron.ballman wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > This diagnostic doesn't tell the user what they've done wrong with the code or why this is a better choice.
> > Yes, but I'm not yet sure what it should say.  Was sorta hoping for a suggestion.  
> Do you have any evidence that this situation happens in practice? I kind of feel like this entire branch could be eliminated from this patch unless it actually catches problems that happen.
Yes, here are a few from clang/lib -- let me know if you think it's worth it or not to keep this:

- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 305293
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaTemplate.cpp
  Message: method 'getAsTemplateDecl' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaTemplate.cpp
    Length: 93
    Offset: 305293
    ReplacementText: dyn_cast_or_null<TemplateTemplateParmDecl>(Name.getAsTemplateDecl())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 153442
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/ASTContext.cpp
  Message: method 'getAsTemplateDecl' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/ASTContext.cpp
    Length: 92
    Offset: 153442
    ReplacementText: dyn_cast_or_null<TypeAliasTemplateDecl>(Template.getAsTemplateDecl())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 97556
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/Expr.cpp
  Message: method 'getMethodDecl' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/AST/Expr.cpp
    Length: 68
    Offset: 97556
    ReplacementText: dyn_cast_or_null<CXXConversionDecl>(MCE->getMethodDecl())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 301950
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaInit.cpp
  Message: method 'get' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/SemaInit.cpp
    Length: 49
    Offset: 301950
    ReplacementText: dyn_cast_or_null<InitListExpr>(CurInit.get())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 14335
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
  Message: method 'operator bool' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
    Length: 57
    Offset: 14335
    ReplacementText: dyn_cast_or_null<CXXTryStmt>(B->getTerminator())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 15997
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
  Message: method 'operator bool' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/lib/Sema/AnalysisBasedWarnings.cpp
    Length: 55
    Offset: 15997
    ReplacementText: dyn_cast_or_null<CXXTryStmt>(B.getTerminator())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 9492
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  Message: method 'sexpr' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
    Length: 39
    Offset: 9492
    ReplacementText: dyn_cast_or_null<til::Undefined>(sexpr())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 9572
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  Message: method 'sexpr' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
    Length: 38
    Offset: 9572
    ReplacementText: dyn_cast_or_null<til::Wildcard>(sexpr())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 9492
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  Message: method 'sexpr' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
    Length: 39
    Offset: 9492
    ReplacementText: dyn_cast_or_null<til::Undefined>(sexpr())
- DiagnosticName: llvm-avoid-cast-in-conditional
  FileOffset: 9572
  FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  Message: method 'sexpr' is called twice and could be expensive
  Replacements:
  - FilePath: /Users/dhinton/projects/llvm_project/monorepo/llvm-project/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
    Length: 38
    Offset: 9572
    ReplacementText: dyn_cast_or_null<til::Wildcard>(sexpr())



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59802/new/

https://reviews.llvm.org/D59802