[llvm] r228934 - CoverageMapping: Bitvectorize code. No functionality change.

[Justin Bogner]

Justin Bogner <mail at justinbogner.com> writes:
> Benjamin Kramer <benny.kra at googlemail.com> writes:
>> -      IsExpandedFile[CR.ExpandedFileID] = true;
>> -  for (unsigned I = 0, E = Function.Filenames.size(); I < E; ++I)
>> -    if (FilenameEquivalence[I] && !IsExpandedFile[I])
>> -      return I;
>> -  return None;
>> +      IsNotExpandedFile[CR.ExpandedFileID] = false;
>> +  IsNotExpandedFile &= FilenameEquivalence;
>> +  int I = IsNotExpandedFile.find_first();
>> +  return I != -1 ? I : None;
>
> You can't use None like this. Since it's just an enum value, the ternary
> here implicitly converts it to int to match I, before the return value
> is converted to Optional. Of course, this means the result is
> Optional<unsigned>(0) instead of None, which is bad.
>
> There are actually two deeper problems this points out:
>
> 1. None is implicitly convertible to int. There's never a time when we
>    want this. Maybe we should change this from "enum NoneType { None }"
>    to "static struct NoneType {} None" or so?

I've changed None to a static instance of an enum class in r229980 so
that code like this errors out at compile time. There was some
discussion on IRC about how to do this even better, so there may yet be
changes to come.