[PATCH] D74639: Add linkage check to ASTNodeImporter::hasSameVisibilityContext and rename to hasSameVisibilityContextAndLinkage

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: clang/lib/AST/ASTImporter.cpp:989
            !Found->hasExternalFormalLinkage();
 }
 
----------------
balazske wrote:
> This would be equivalent but more clear code (formatting is probably not correct here):
> ```
>   if (Found->getLinkageInternal() != From->getLinkageInternal())
>     return false;
> 
>   if (From->hasExternalFormalLinkage())
>     return true;
> 
>   return Importer.GetFromTU(Found) == From->getTranslationUnitDecl() &&
>          From->isInAnonymousNamespace() == Found->isInAnonymousNamespace();
> ```
Yeah, this function is not easy to follow, so this could benefit from a refactoring, I agree. But that is somewhat orthogonal to this change in my opinion.


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

https://reviews.llvm.org/D74639