r184048 - Updated the support for contextual conversion tweaks (n3323) with a previously overlooked part: implicitly converting array sizes to size_t, rather than contextually converting them to some unique type.

Renato Golin renato.golin at linaro.org
Sun Jun 16 06:10:41 PDT 2013


On 16 June 2013 05:47, Larisse Voufo <lvoufo at google.com> wrote:

> Yeah. That's weird. I am not exactly sure how the specific changes I made
> could cause that kind of damage, especially when I successfully ran all the
> regression tests before commiting...
>

Benjamin hinted that the bug was only in 32-bits systems. If you only
tested on a 64-bits, than it's understandable that you broke what you
haven't tested. That's what the buildbots are for. ;)




> I was informed that buildbots haven't been the most reliable lately(?)
> However, I'm still not quite sure how to tell "flakes" from non-"flakes".
> Any idea?
>

That's easy. Whenever you receive an email saying you broke a buildbot, you
look at the error and see if it's in an area that you have changed. If it
is, try to fix it as soon as possible. If not, leave to the buildbot
maintainer to chase the bug. What you shouldn't do is ignore the message.

Random failures occur, but they tend to be compilation errors, for instance
for lack of memory or timeout. Sometimes, MCJIT tests fail randomly, and
we're trying to get those fixed as well. So, if the compiler error is
something like "unrecoverable error" with no error message, it's probably
not your fault. If the error is in a test that has nothing to do with your
change (ex. MCJIT failures after a C++11 change), it should be safe to
ignore, too. But we appreciate



> I just noticed that a couple of the failures have to do with a test case's
> expected warning message not matching some architecture-specific data. I
> have fixed the test case in question, and did some code-base resetting
> (directly mirroring the repository) over here. Hopefully that will fix at
> least a few of these...
>
> Thanks,
> -- Larisse.
>
>
>> cheers,
>> -renato
>>
>>
>> On 15 June 2013 21:17, Larisse Voufo <lvoufo at google.com> wrote:
>>
>>> Author: lvoufo
>>> Date: Sat Jun 15 15:17:46 2013
>>> New Revision: 184048
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=184048&view=rev
>>> Log:
>>> Updated the support for contextual conversion tweaks (n3323) with a
>>> previously overlooked part: implicitly converting array sizes to size_t,
>>> rather than contextually converting them to some unique type.
>>>
>>> Modified:
>>>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>     cfe/trunk/include/clang/Sema/Sema.h
>>>     cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>>     cfe/trunk/test/SemaCXX/cxx98-compat-pedantic.cpp
>>>
>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=184048&r1=184047&r2=184048&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Jun 15
>>> 15:17:46 2013
>>> @@ -36,15 +36,15 @@ def err_typecheck_converted_constant_exp
>>>  def err_typecheck_converted_constant_expression_disallowed : Error<
>>>    "conversion from %0 to %1 is not allowed in a converted constant
>>> expression">;
>>>  def err_expr_not_cce : Error<
>>> -  "%select{case value|enumerator value|non-type template argument}0 "
>>> +  "%select{case value|enumerator value|non-type template argument|array
>>> size}0 "
>>>    "is not a constant expression">;
>>>  def err_cce_narrowing : ExtWarn<
>>> -  "%select{case value|enumerator value|non-type template argument}0 "
>>> +  "%select{case value|enumerator value|non-type template argument|array
>>> size}0 "
>>>    "%select{cannot be narrowed from type %2 to %3|"
>>>    "evaluates to %2, which cannot be narrowed to type %3}1">,
>>>    InGroup<CXX11Narrowing>, DefaultError;
>>>  def err_cce_narrowing_sfinae : Error<
>>> -  "%select{case value|enumerator value|non-type template argument}0 "
>>> +  "%select{case value|enumerator value|non-type template argument|array
>>> size}0 "
>>>    "%select{cannot be narrowed from type %2 to %3|"
>>>    "evaluates to %2, which cannot be narrowed to type %3}1">;
>>>  def err_ice_not_integral : Error<
>>>
>>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=184048&r1=184047&r2=184048&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>>> +++ cfe/trunk/include/clang/Sema/Sema.h Sat Jun 15 15:17:46 2013
>>> @@ -1938,9 +1938,10 @@ public:
>>>
>>>    /// Contexts in which a converted constant expression is required.
>>>    enum CCEKind {
>>> -    CCEK_CaseValue,  ///< Expression in a case label.
>>> -    CCEK_Enumerator, ///< Enumerator value with fixed underlying type.
>>> -    CCEK_TemplateArg ///< Value of a non-type template parameter.
>>> +    CCEK_CaseValue,   ///< Expression in a case label.
>>> +    CCEK_Enumerator,  ///< Enumerator value with fixed underlying type.
>>> +    CCEK_TemplateArg, ///< Value of a non-type template parameter.
>>> +    CCEK_NewExpr      ///< Constant expression in a
>>> noptr-new-declarator.
>>>    };
>>>    ExprResult CheckConvertedConstantExpression(Expr *From, QualType T,
>>>                                                llvm::APSInt &Value,
>>> CCEKind CCE);
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=184048&r1=184047&r2=184048&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Sat Jun 15 15:17:46 2013
>>> @@ -1017,10 +1017,23 @@ Sema::ActOnCXXNew(SourceLocation StartLo
>>>        DeclaratorChunk::ArrayTypeInfo &Array = D.getTypeObject(I).Arr;
>>>        if (Expr *NumElts = (Expr *)Array.NumElts) {
>>>          if (!NumElts->isTypeDependent() &&
>>> !NumElts->isValueDependent()) {
>>> -          Array.NumElts
>>> -            = VerifyIntegerConstantExpression(NumElts, 0,
>>> -
>>>  diag::err_new_array_nonconst)
>>> -                .take();
>>> +          if (getLangOpts().CPlusPlus1y) {
>>> +           // C++1y [expr.new]p6: Every constant-expression in a
>>> noptr-new-declarator
>>> +           //   shall be a converted constant expression (5.19) of type
>>> std::size_t
>>> +           //   and shall evaluate to a strictly positive value.
>>> +            unsigned IntWidth = Context.getTargetInfo().getIntWidth();
>>> +            assert(IntWidth && "Builtin type of size 0?");
>>> +            llvm::APSInt Value(IntWidth);
>>> +            Array.NumElts
>>> +             = CheckConvertedConstantExpression(NumElts,
>>> Context.getSizeType(), Value,
>>> +                                                CCEK_NewExpr)
>>> +                 .take();
>>> +          } else {
>>> +            Array.NumElts
>>> +              = VerifyIntegerConstantExpression(NumElts, 0,
>>> +
>>>  diag::err_new_array_nonconst)
>>> +                  .take();
>>> +          }
>>>            if (!Array.NumElts)
>>>              return ExprError();
>>>          }
>>> @@ -1183,65 +1196,81 @@ Sema::BuildCXXNew(SourceRange Range, boo
>>>    // C++1y [expr.new]p6: The expression [...] is implicitly converted to
>>>    //   std::size_t. (FIXME)
>>>    if (ArraySize && !ArraySize->isTypeDependent()) {
>>> -    class SizeConvertDiagnoser : public ICEConvertDiagnoser {
>>> -      Expr *ArraySize;
>>> -
>>> -    public:
>>> -      SizeConvertDiagnoser(Expr *ArraySize)
>>> -          : ICEConvertDiagnoser(/*AllowScopedEnumerations*/false,
>>> false, false),
>>> -            ArraySize(ArraySize) {}
>>> -
>>> -      virtual SemaDiagnosticBuilder diagnoseNotInt(Sema &S,
>>> SourceLocation Loc,
>>> -                                                   QualType T) {
>>> -        return S.Diag(Loc, diag::err_array_size_not_integral)
>>> -                 << S.getLangOpts().CPlusPlus11 << T;
>>> -      }
>>> -
>>> -      virtual SemaDiagnosticBuilder diagnoseIncomplete(
>>> -          Sema &S, SourceLocation Loc, QualType T) {
>>> -        return S.Diag(Loc, diag::err_array_size_incomplete_type)
>>> -                 << T << ArraySize->getSourceRange();
>>> -      }
>>> -
>>> -      virtual SemaDiagnosticBuilder diagnoseExplicitConv(
>>> -          Sema &S, SourceLocation Loc, QualType T, QualType ConvTy) {
>>> -        return S.Diag(Loc, diag::err_array_size_explicit_conversion) <<
>>> T << ConvTy;
>>> -      }
>>> -
>>> -      virtual SemaDiagnosticBuilder noteExplicitConv(
>>> -          Sema &S, CXXConversionDecl *Conv, QualType ConvTy) {
>>> -        return S.Diag(Conv->getLocation(),
>>> diag::note_array_size_conversion)
>>> -                 << ConvTy->isEnumeralType() << ConvTy;
>>> -      }
>>> -
>>> -      virtual SemaDiagnosticBuilder diagnoseAmbiguous(
>>> -          Sema &S, SourceLocation Loc, QualType T) {
>>> -        return S.Diag(Loc, diag::err_array_size_ambiguous_conversion)
>>> << T;
>>> -      }
>>> -
>>> -      virtual SemaDiagnosticBuilder noteAmbiguous(
>>> -          Sema &S, CXXConversionDecl *Conv, QualType ConvTy) {
>>> -        return S.Diag(Conv->getLocation(),
>>> diag::note_array_size_conversion)
>>> -                 << ConvTy->isEnumeralType() << ConvTy;
>>> -      }
>>> +    ExprResult ConvertedSize;
>>> +    if (getLangOpts().CPlusPlus1y) {
>>> +      unsigned IntWidth = Context.getTargetInfo().getIntWidth();
>>> +      assert(IntWidth && "Builtin type of size 0?");
>>> +      llvm::APSInt Value(IntWidth);
>>> +      ConvertedSize = PerformImplicitConversion(ArraySize,
>>> Context.getSizeType(),
>>> +                                               AA_Converting);
>>> +
>>> +      if (!isSFINAEContext())
>>> +       // Diagnose the compatibility of this conversion.
>>> +       Diag(StartLoc, diag::warn_cxx98_compat_array_size_conversion)
>>> +         << ArraySize->getType() << 0 << Context.getSizeType();
>>> +    } else {
>>> +      class SizeConvertDiagnoser : public ICEConvertDiagnoser {
>>> +      protected:
>>> +        Expr *ArraySize;
>>> +
>>> +      public:
>>> +        SizeConvertDiagnoser(Expr *ArraySize)
>>> +            : ICEConvertDiagnoser(/*AllowScopedEnumerations*/false,
>>> false, false),
>>> +              ArraySize(ArraySize) {}
>>> +
>>> +        virtual SemaDiagnosticBuilder diagnoseNotInt(Sema &S,
>>> SourceLocation Loc,
>>> +                                                     QualType T) {
>>> +          return S.Diag(Loc, diag::err_array_size_not_integral)
>>> +                   << S.getLangOpts().CPlusPlus11 << T;
>>> +        }
>>> +
>>> +        virtual SemaDiagnosticBuilder diagnoseIncomplete(
>>> +            Sema &S, SourceLocation Loc, QualType T) {
>>> +          return S.Diag(Loc, diag::err_array_size_incomplete_type)
>>> +                   << T << ArraySize->getSourceRange();
>>> +        }
>>> +
>>> +        virtual SemaDiagnosticBuilder diagnoseExplicitConv(
>>> +            Sema &S, SourceLocation Loc, QualType T, QualType ConvTy) {
>>> +          return S.Diag(Loc, diag::err_array_size_explicit_conversion)
>>> << T << ConvTy;
>>> +        }
>>> +
>>> +        virtual SemaDiagnosticBuilder noteExplicitConv(
>>> +            Sema &S, CXXConversionDecl *Conv, QualType ConvTy) {
>>> +          return S.Diag(Conv->getLocation(),
>>> diag::note_array_size_conversion)
>>> +                   << ConvTy->isEnumeralType() << ConvTy;
>>> +        }
>>> +
>>> +        virtual SemaDiagnosticBuilder diagnoseAmbiguous(
>>> +            Sema &S, SourceLocation Loc, QualType T) {
>>> +          return S.Diag(Loc, diag::err_array_size_ambiguous_conversion)
>>> << T;
>>> +        }
>>> +
>>> +        virtual SemaDiagnosticBuilder noteAmbiguous(
>>> +            Sema &S, CXXConversionDecl *Conv, QualType ConvTy) {
>>> +          return S.Diag(Conv->getLocation(),
>>> diag::note_array_size_conversion)
>>> +                   << ConvTy->isEnumeralType() << ConvTy;
>>> +        }
>>>
>>> -      virtual SemaDiagnosticBuilder diagnoseConversion(
>>> -          Sema &S, SourceLocation Loc, QualType T, QualType ConvTy) {
>>> -        return S.Diag(Loc,
>>> -                      S.getLangOpts().CPlusPlus11
>>> -                        ? diag::warn_cxx98_compat_array_size_conversion
>>> -                        : diag::ext_array_size_conversion)
>>> -                 << T << ConvTy->isEnumeralType() << ConvTy;
>>> -      }
>>> -    } SizeDiagnoser(ArraySize);
>>> +        virtual SemaDiagnosticBuilder diagnoseConversion(
>>> +            Sema &S, SourceLocation Loc, QualType T, QualType ConvTy) {
>>> +          return S.Diag(Loc,
>>> +                        S.getLangOpts().CPlusPlus11
>>> +                          ?
>>> diag::warn_cxx98_compat_array_size_conversion
>>> +                          : diag::ext_array_size_conversion)
>>> +                   << T << ConvTy->isEnumeralType() << ConvTy;
>>> +        }
>>> +      } SizeDiagnoser(ArraySize);
>>>
>>> -    ExprResult ConvertedSize
>>> -      = PerformContextualImplicitConversion(StartLoc, ArraySize,
>>> SizeDiagnoser);
>>> +      ConvertedSize = PerformContextualImplicitConversion(StartLoc,
>>> ArraySize,
>>> +
>>>  SizeDiagnoser);
>>> +    }
>>>      if (ConvertedSize.isInvalid())
>>>        return ExprError();
>>>
>>>      ArraySize = ConvertedSize.take();
>>>      QualType SizeType = ArraySize->getType();
>>> +
>>>      if (!SizeType->isIntegralOrUnscopedEnumerationType())
>>>        return ExprError();
>>>
>>>
>>> Modified: cfe/trunk/test/SemaCXX/cxx98-compat-pedantic.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx98-compat-pedantic.cpp?rev=184048&r1=184047&r2=184048&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/SemaCXX/cxx98-compat-pedantic.cpp (original)
>>> +++ cfe/trunk/test/SemaCXX/cxx98-compat-pedantic.cpp Sat Jun 15 15:17:46
>>> 2013
>>> @@ -1,10 +1,10 @@
>>> -// RUN: %clang_cc1 -fsyntax-only -std=c++1y -DCXX1Y
>>> -Wc++98-compat-pedantic -verify %s
>>> -// RUN: %clang_cc1 -fsyntax-only -std=c++1y -DCXX1Y -Wc++98-compat
>>> -Werror %s
>>> +// RUN: %clang_cc1 -fsyntax-only -std=c++1y -DCXX1Y
>>> -Wc++98-compat-pedantic -verify %s -DCXX1Y2
>>> +// RUN: %clang_cc1 -fsyntax-only -std=c++1y -DCXX1Y -Wc++98-compat
>>> -Werror %s -DCXX1Y2
>>>  // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat-pedantic
>>> -verify %s
>>>  // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat -Werror %s
>>>  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -Werror %s
>>>
>>> -// RUN: %clang_cc1 -fsyntax-only -std=c++1y -Wc++98-compat-pedantic
>>> -verify %s -Wno-c++98-c++11-compat-pedantic
>>> +// RUN: %clang_cc1 -fsyntax-only -std=c++1y -Wc++98-compat-pedantic
>>> -verify %s -Wno-c++98-c++11-compat-pedantic -DCXX1Y2
>>>
>>>  // -Wc++98-compat-pedantic warns on C++11 features which we accept
>>> without a
>>>  // warning in C++98 mode.
>>> @@ -32,7 +32,12 @@ void *FnVoidPtr = (void*)&dlsym; // expe
>>>  struct ConvertToInt {
>>>    operator int();
>>>  };
>>> -int *ArraySizeConversion = new int[ConvertToInt()]; // expected-warning
>>> {{implicit conversion from array size expression of type 'ConvertToInt' to
>>> integral type 'int' is incompatible with C++98}}
>>> +int *ArraySizeConversion = new int[ConvertToInt()];
>>> +#ifdef CXX1Y2
>>> +// expected-warning at -2 {{implicit conversion from array size
>>> expression of type 'ConvertToInt' to integral type 'unsigned long' is
>>> incompatible with C++98}}
>>> +#else
>>> +// expected-warning at -4 {{implicit conversion from array size
>>> expression of type 'ConvertToInt' to integral type 'int' is incompatible
>>> with C++98}}
>>> +#endif
>>>
>>>  template<typename T> class ExternTemplate {};
>>>  extern template class ExternTemplate<int>; // expected-warning {{extern
>>> templates are incompatible with C++98}}
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130616/44bf2781/attachment.html>


More information about the cfe-commits mailing list