[PATCH] unsigned count() for ADT

[Duncan P. N. Exon Smith]

> On 2014 Jun 10, at 08:24, David Blaikie <dblaikie at gmail.com> wrote:
> 
> ================
> Comment at: lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp:311
> @@ -310,3 +310,3 @@
>   const Function *GV = MF->getFunction();
> -  assert(FnDebugInfo.count(GV) == true);
> +  assert(FnDebugInfo.count(GV) == 1);
>   assert(CurFn == &FnDebugInfo[GV]);
> ----------------
> It's common to write a "does this container contain this thing" as simply boolean testing the result of 'count':
> 
> assert(FnDebugInfo.count(GV));
> 
> Personally this is easier for me to read because I don't have to worry about "is this container a multimap and are they testing that there's /exactly/ one element" (and writing it as "> 0" when it's a single map is also a little confusing, even though it's semantically equivalent to the non-zero count test above)
> 
> Not a requirement, just a though - maybe others feel more strongly.

FWIW, I agree with David here.  In my experience, `count()` is almost
always used as a `bool`, and it's strange (and likely bug prone) to use
a comparison instead.

> 
> ================
> Comment at: lib/IR/Metadata.cpp:666
> @@ -665,3 +665,3 @@
>   assert((hasMetadataHashEntry() ==
> -          getContext().pImpl->MetadataStore.count(this)) &&
> +          (getContext().pImpl->MetadataStore.count(this) == 1)) &&
>          "HasMetadata bit out of date!");
> ----------------
> same here, though I guess this is a bit more interesting as in this case the bool on the LHS would be converted to an int and compared... but you're == 1, so it is the same whether it's a multimap or not (ie: it's only true if there's exactly one thing)... not sure how I'd prefer to write this one.
> 
> http://reviews.llvm.org/D4018
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits