[PATCH] D19468: Disallow duplication of imported entities (improved implementation)

[David Blaikie via llvm-commits]

I'm still vaguely thinking we should be doing this in the frontend instead.

Could you remind me what problems happen if there are duplicates? And why
it was hard to detect duplicates in the frontend? (why we even ever visited
the same using decl more than once?)

On Sun, Apr 24, 2016 at 6:47 AM, Amjad Aboud via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> aaboud created this revision.
> aaboud added reviewers: dexonsmith, dblaikie, aprantl.
> aaboud added a subscriber: llvm-commits.
>
> Following Duncan suggestion of improving code to disallow duplication of
> imported entities from commit: http://reviews.llvm.org/rL263379
> By simply filter the AllImportedModules list at DIBuilder::finalize
> function.
>
> Note: LIT test was committed in revision 263379.
>
> http://reviews.llvm.org/D19468
>
> Files:
>   lib/IR/DIBuilder.cpp
>
> Index: lib/IR/DIBuilder.cpp
> ===================================================================
> --- lib/IR/DIBuilder.cpp
> +++ lib/IR/DIBuilder.cpp
> @@ -119,10 +119,16 @@
>    if (!AllGVs.empty())
>      CUNode->replaceGlobalVariables(MDTuple::get(VMContext, AllGVs));
>
> -  if (!AllImportedModules.empty())
> -    CUNode->replaceImportedEntities(MDTuple::get(
> -        VMContext, SmallVector<Metadata *, 16>(AllImportedModules.begin(),
> -
>  AllImportedModules.end())));
> +  // Create imported entities list without duplications.
> +  SmallVector<Metadata *, 16> ImportedModules;
> +  SmallPtrSet<Metadata *, 16> AlreadyImported;
> +  std::remove_copy_if(AllImportedModules.begin(),
> AllImportedModules.end(),
> +    std::back_inserter(ImportedModules),
> +    [&AlreadyImported](TrackingMDNodeRef &Ref) {
> +    return !AlreadyImported.insert(Ref.get()).second;
> +  });
> +  if (!ImportedModules.empty())
> +    CUNode->replaceImportedEntities(MDTuple::get(VMContext,
> ImportedModules));
>
>    // Now that all temp nodes have been replaced or deleted, resolve
> remaining
>    // cycles.
> @@ -170,12 +176,8 @@
>  createImportedModule(LLVMContext &C, dwarf::Tag Tag, DIScope *Context,
>                       Metadata *NS, unsigned Line, StringRef Name,
>                       SmallVectorImpl<TrackingMDNodeRef>
> &AllImportedModules) {
> -  unsigned EntitiesCount = C.pImpl->DIImportedEntitys.size();
>    auto *M = DIImportedEntity::get(C, Tag, Context, DINodeRef(NS), Line,
> Name);
> -  if (EntitiesCount < C.pImpl->DIImportedEntitys.size())
> -    // A new Imported Entity was just added to the context.
> -    // Add it to the Imported Modules list.
> -    AllImportedModules.emplace_back(M);
> +  AllImportedModules.emplace_back(M);
>    return M;
>  }
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160425/8b1609c4/attachment.html>