Use of unnecessary conditions checks Inside clang/lib/AST/ASTImporter.cpp

Subprojects LLDB
#1

I was looking into the codebase of clang/lib/AST/ASTImporter.cpp, at the line no. 5872

  if (DTemplated->isThisDeclarationADefinition() &&
      !ToTemplated->isThisDeclarationADefinition()) {
    // FIXME: Import definition!
  }

there is similar code at line no. 5589 also. I’m curious about what is the need of Import definition and this fixme is since 2013 don’t know why , It’s more like it is irrelevant and then checking the conditions doesn’t make sense, ultimately might be it’ll increase the time because of the irrelevant checking . If there is no solid need of these conditions , we should remove this ! Want to know more about it , Please comment your views.
CC: @shafik , @AaronBallman , @martong

#2

(Trying this reply out from email instead of the Discourse web interface. If something looks wrong to you, let me know!)

As best I can tell, those FIXMEs have been there since 2013 with: https://github.com/llvm/llvm-project/commit/39a1e507ff0bef4bd6b2fdbab4e38583d2679617. I’m not certain the circumstances under which conditions further work is needed here. I didn’t spot any test cases in that commit which related to PCH.