[llvm] r229980 - Disallow implicit conversions from None to integer types

[David Blaikie]

On Feb 20, 2015 4:26 AM, "Andrea Di Biagio" <andrea.dibiagio at gmail.com>
wrote:
>
> Hi Justin,
>
> Just for your info. After this change I get tons of new warnings
> because of ‘llvm::None’ defined but not used [-Wunused-variable].
>
> Example:
>
> In file included from llvm/include/llvm/ADT/ArrayRef.h:13:0,
>                  llvm/include/llvm/ADT/APInt.h:19,
>                  llvm/include/llvm/ADT/APFloat.h:20,
>                  llvm/lib/Support/APFloat.cpp:15:
> llvm/include/llvm/ADT/None.h:23:17: warning: ‘llvm::None’ defined but
> not used [-Wunused-variable]
>  static NoneType None;
>                  ^

Attribute unused? (Used?) I never remember which.

We should check the constexpr rules/option perhaps.

>
> I am using gcc version 4.8.2 (Ubuntu 4.8.2-19ubuntu1).
>
> -Andrea
>
> On Fri, Feb 20, 2015 at 7:28 AM, Justin Bogner <mail at justinbogner.com>
wrote:
> > Author: bogner
> > Date: Fri Feb 20 01:28:28 2015
> > New Revision: 229980
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=229980&view=rev
> > Log:
> > Disallow implicit conversions from None to integer types
> >
> > This fixes an error introduced in r228934 where None was converted to
> > an int instead of the int being converted to an Optional as intended.
> > We make that sort of mistake a compile error by changing NoneType into
> > a scoped enum.
> >
> > Finally, provide a static NoneType called None to avoid forcing all
> > users to spell it NoneType::None.
> >
> > Modified:
> >     llvm/trunk/include/llvm/ADT/None.h
> >     llvm/trunk/lib/ProfileData/CoverageMapping.cpp
> >
> > Modified: llvm/trunk/include/llvm/ADT/None.h
> > URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/None.h?rev=229980&r1=229979&r2=229980&view=diff
> >
==============================================================================
> > --- llvm/trunk/include/llvm/ADT/None.h (original)
> > +++ llvm/trunk/include/llvm/ADT/None.h Fri Feb 20 01:28:28 2015
> > @@ -19,9 +19,8 @@
> >  namespace llvm {
> >  /// \brief A simple null object to allow implicit construction of
Optional<T>
> >  /// and similar types without having to spell out the specialization's
name.
> > -enum NoneType {
> > -  None
> > -};
> > +enum class NoneType { None };
> > +static NoneType None;
> >  }
> >
> >  #endif
> >
> > Modified: llvm/trunk/lib/ProfileData/CoverageMapping.cpp
> > URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/CoverageMapping.cpp?rev=229980&r1=229979&r2=229980&view=diff
> >
==============================================================================
> > --- llvm/trunk/lib/ProfileData/CoverageMapping.cpp (original)
> > +++ llvm/trunk/lib/ProfileData/CoverageMapping.cpp Fri Feb 20 01:28:28
2015
> > @@ -361,7 +361,9 @@ static Optional<unsigned> findMainViewFi
> >        IsNotExpandedFile[CR.ExpandedFileID] = false;
> >    IsNotExpandedFile &= FilenameEquivalence;
> >    int I = IsNotExpandedFile.find_first();
> > -  return I != -1 ? I : None;
> > +  if (I == -1)
> > +    return None;
> > +  return I;
> >  }
> >
> >  static Optional<unsigned> findMainViewFileID(const FunctionRecord
&Function) {
> > @@ -370,7 +372,9 @@ static Optional<unsigned> findMainViewFi
> >      if (CR.Kind == CounterMappingRegion::ExpansionRegion)
> >        IsNotExpandedFile[CR.ExpandedFileID] = false;
> >    int I = IsNotExpandedFile.find_first();
> > -  return I != -1 ? I : None;
> > +  if (I == -1)
> > +    return None;
> > +  return I;
> >  }
> >
> >  /// Sort a nested sequence of regions from a single file.
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150220/aeee0e9c/attachment.html>