[PATCH] D55658: Avoid Decl::getASTContext in hot paths where possible, Part 1

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rsmith.
aaron.ballman added a subscriber: rsmith.
aaron.ballman added a comment.

I'd like @rsmith's opinion on whether this is a good churn or not. I think it's mostly reasonable, but it's also a lot of changes for identical behavior and in some cases the changes are a bit unfortunate.



================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:4504
                                                         FunctionDecl)) {
-  return Node.isThisDeclarationADefinition();
 }
----------------
I don't care for this refactoring -- the new code repeats the types from the function definition and is considerably harder to read.


================
Comment at: lib/AST/Decl.cpp:2378
+  assert(D);
+  if (auto *Def = D->getDefinition(Context))
+    return Def;
----------------
Bad use of `auto`, though it is used above so maybe it should stay for local consistency.


================
Comment at: lib/Sema/SemaDecl.cpp:2849
+static std::pair<diag::kind, SourceLocation>
+getNoteDiagForInvalidRedeclaration(const ASTContext &Context, const T *Old,
+                                   const T *New) {
----------------
I'm not keen on the code duplication. :-(


Repository:
  rC Clang

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

https://reviews.llvm.org/D55658