[PATCH] D15991: Remove unnecessary .get() on smart pointers in isa, cast, and dyn_cast.

[Justin Bogner via llvm-commits]

David Blaikie via llvm-commits <llvm-commits at lists.llvm.org> writes:
> dblaikie added a comment.
>
> Looks good - a few optional comments. Feel free to commit with or without the suggested changes, as you see fit.
>
>
> ================
> Comment at: lib/DebugInfo/PDB/PDBContext.cpp:104
> @@ -103,3 +103,3 @@
>          Session->findSymbolByAddress(Address, PDB_SymType::PublicSymbol);
> -    if (auto PS = dyn_cast_or_null<PDBSymbolPublicSymbol>(PublicSym.get()))
> +    if (auto PS = dyn_cast_or_null<PDBSymbolPublicSymbol>(PublicSym))
>        return PS->getName();
> ----------------
> Might want to ensure we have '*'s on the LHS especially when casting
> from a smart pointer - might be extra confusing getting a raw pointer
> from a smart pointer.

This is actually kind of scary. After thinking about this, I don't like
dyn_cast/dyn_cast_or_null working like this at all. The pointer being
escaped is too subtle.

I'll comment to this effect on D15910.

> ================
> Comment at: lib/Object/ELFYAML.cpp:716
> @@ -715,3 +715,3 @@
>        Section.reset(new ELFYAML::RelocationSection());
> -    sectionMapping(IO, *cast<ELFYAML::RelocationSection>(Section.get()));
> +    sectionMapping(IO, *cast<ELFYAML::RelocationSection>(Section));
>      break;
> ----------------
> These might be more obvious (then there's no interesting smart pointer to raw pointer conversions, etc) if the * was inside the cast instead of outside - but not sure.
>
> ================
> Comment at: tools/llvm-pdbdump/ClassDefinitionDumper.cpp:86
> @@ -85,3 +85,3 @@
>  
> -    if (auto Func = dyn_cast<PDBSymbolFunc>(Child.get())) {
> +    if (auto Func = dyn_cast<PDBSymbolFunc>(Child)) {
>        if (Func->isCompilerGenerated() && opts::ExcludeCompilerGenerated)
> ----------------
> some more missing '*' (I know, not introduced by this patch, but possibly exacerbated)
>
>
> http://reviews.llvm.org/D15991
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits