[PATCH] D55280: [CTU] Remove redundant error check

Balázs Kéri via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 5 02:23:28 PST 2018


balazske added inline comments.


================
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:121
+  /// \return Returns a pointer to the ASTUnit that contains the definition of
+  /// the looked up function. The pointer should not be a nullptr.
   ///
----------------
Return value description still not accurate: It returns the `Expected` object, not the pointer itself.


================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:147
 
 llvm::Expected<const FunctionDecl *>
 CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD,
----------------
Szelethus wrote:
> Would it be worth to add a comment that this function never returns with `nullptr` on success?
Even on failure not. Failure should be `Error`, success is a non-null pointer.


================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:238
   }
+  assert(Unit);
   return Unit;
----------------
Add explanation text to the assert?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55280





More information about the cfe-commits mailing list