[libcxx-commits] [PATCH] D130308: [clang] extend getCommonSugaredType to merge sugar nodes

[Erich Keane via Phabricator via libcxx-commits]

erichkeane accepted this revision.
erichkeane added a comment.

I've got concerns about the interface not being clear on null-ability, and THIS patch seems to move us toward nulls being disallowed, so I'd like that formalized when possible (either via asserts or, better, by switching to references when possible).  Otherwise, LG.



================
Comment at: clang/lib/AST/ASTContext.cpp:12141
 static Decl *getCommonDecl(Decl *X, Decl *Y) {
-  if (X == Y)
-    return X;
-  assert(declaresSameEntity(X, Y));
+  if (!declaresSameEntity(X, Y))
+    return nullptr;
----------------
same concerns about null here.  I find myself wondering if this 'getCommonX' should take/return references when possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130308