Looks fixed. Thank a lot for investigating! > On Sep 3, 2015, at 9:21 PM, Steven Wu via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > Sorry I just get back to my computer. I will watch the bots going. I will try > investigating if it is still failing. > As far as I know, there is no special settingl about the bots and all our > bots are complaining about it. > > Steven > >> On Sep 3, 2015, at 9:09 PM, Richard Smith <rich...@metafoo.co.uk >> <mailto:rich...@metafoo.co.uk>> wrote: >> >> OK, should be fixed in r246836. >> >> On Thu, Sep 3, 2015 at 7:50 PM, Richard Smith <rich...@metafoo.co.uk >> <mailto:rich...@metafoo.co.uk>> wrote: >> On Thu, Sep 3, 2015 at 6:21 PM, Steven Wu via cfe-commits >> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >> In case you didn’t get an email from the failure because it was overshadowed >> by the previous error. This commit seems to break the green dragon bots: >> http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/12990/testReport/junit/Clang/Sema/attr_flag_enum_c/ >> >> <http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/12990/testReport/junit/Clang/Sema/attr_flag_enum_c/> >> >> I have no idea what's wrong here; is there anything unusual about that bot >> that might give me a clue? (This bot: >> http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/4637 >> <http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/4637> seems >> to be failing the same way.) >> >> Thanks >> >> Steven >> >>> On Sep 3, 2015, at 6:03 PM, Richard Smith via cfe-commits >>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>> >>> Author: rsmith >>> Date: Thu Sep 3 20:03:03 2015 >>> New Revision: 246830 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=246830&view=rev >>> <http://llvm.org/viewvc/llvm-project?rev=246830&view=rev> >>> Log: >>> Fix a potential APInt memory leak when using __attribute__((flag_enum)), and >>> simplify the implementation a bit. >>> >>> Modified: >>> cfe/trunk/include/clang/Basic/Attr.td >>> cfe/trunk/include/clang/Sema/Sema.h >>> cfe/trunk/lib/Sema/SemaDecl.cpp >>> cfe/trunk/lib/Sema/SemaStmt.cpp >>> >>> Modified: cfe/trunk/include/clang/Basic/Attr.td >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246830&r1=246829&r2=246830&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246830&r1=246829&r2=246830&view=diff> >>> ============================================================================== >>> --- cfe/trunk/include/clang/Basic/Attr.td (original) >>> +++ cfe/trunk/include/clang/Basic/Attr.td Thu Sep 3 20:03:03 2015 >>> @@ -747,18 +747,6 @@ def FlagEnum : InheritableAttr { >>> let Subjects = SubjectList<[Enum]>; >>> let Documentation = [FlagEnumDocs]; >>> let LangOpts = [COnly]; >>> - let AdditionalMembers = [{ >>> -private: >>> - llvm::APInt FlagBits; >>> -public: >>> - llvm::APInt &getFlagBits() { >>> - return FlagBits; >>> - } >>> - >>> - const llvm::APInt &getFlagBits() const { >>> - return FlagBits; >>> - } >>> -}]; >>> } >>> >>> def Flatten : InheritableAttr { >>> >>> Modified: cfe/trunk/include/clang/Sema/Sema.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=246830&r1=246829&r2=246830&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=246830&r1=246829&r2=246830&view=diff> >>> ============================================================================== >>> --- cfe/trunk/include/clang/Sema/Sema.h (original) >>> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Sep 3 20:03:03 2015 >>> @@ -903,6 +903,10 @@ public: >>> /// for C++ records. >>> llvm::FoldingSet<SpecialMemberOverloadResult> SpecialMemberCache; >>> >>> + /// \brief A cache of the flags available in enumerations with the >>> flag_bits >>> + /// attribute. >>> + mutable llvm::DenseMap<const EnumDecl*, llvm::APInt> FlagBitsCache; >>> + >>> /// \brief The kind of translation unit we are processing. >>> /// >>> /// When we're processing a complete translation unit, Sema will perform >>> >>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=246830&r1=246829&r2=246830&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=246830&r1=246829&r2=246830&view=diff> >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Sep 3 20:03:03 2015 >>> @@ -14017,14 +14017,21 @@ static void CheckForDuplicateEnumValues( >>> bool >>> Sema::IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val, >>> bool AllowMask) const { >>> - FlagEnumAttr *FEAttr = ED->getAttr<FlagEnumAttr>(); >>> - assert(FEAttr && "looking for value in non-flag enum"); >>> + assert(ED->hasAttr<FlagEnumAttr>() && "looking for value in non-flag >>> enum"); >>> >>> - llvm::APInt FlagMask = ~FEAttr->getFlagBits(); >>> - unsigned Width = FlagMask.getBitWidth(); >>> + auto R = FlagBitsCache.insert(std::make_pair(ED, llvm::APInt())); >>> + llvm::APInt &FlagBits = R.first->second; >>> >>> - // We will try a zero-extended value for the regular check first. >>> - llvm::APInt ExtVal = Val.zextOrSelf(Width); >>> + if (R.second) { >>> + for (auto *E : ED->enumerators()) { >>> + const auto &Val = E->getInitVal(); >>> + // Only single-bit enumerators introduce new flag values. >>> + if (Val.isPowerOf2()) >>> + FlagBits = FlagBits.zextOrSelf(Val.getBitWidth()) | Val; >>> + } >>> + } >>> + >>> + llvm::APInt FlagMask = ~FlagBits.zextOrTrunc(Val.getBitWidth()); >>> >>> // A value is in a flag enum if either its bits are a subset of the enum's >>> // flag bits (the first condition) or we are allowing masks and the same >>> is >>> @@ -14034,26 +14041,10 @@ Sema::IsValueInFlagEnum(const EnumDecl * >>> // While it's true that any value could be used as a mask, the assumption >>> is >>> // that a mask will have all of the insignificant bits set. Anything else >>> is >>> // likely a logic error. >>> - if (!(FlagMask & ExtVal)) >>> + if (!(FlagMask & Val) || >>> + (AllowMask && !(FlagMask & ~Val))) >>> return true; >>> >>> - if (AllowMask) { >>> - // Try a one-extended value instead. This can happen if the enum is >>> wider >>> - // than the constant used, in C with extensions to allow for wider >>> enums. >>> - // The mask will still have the correct behaviour, so we give the user >>> the >>> - // benefit of the doubt. >>> - // >>> - // FIXME: This heuristic can cause weird results if the enum was >>> extended >>> - // to a larger type and is signed, because then bit-masks of smaller >>> types >>> - // that get extended will fall out of range (e.g. ~0x1u). We currently >>> don't >>> - // detect that case and will get a false positive for it. In most >>> cases, >>> - // though, it can be fixed by making it a signed type (e.g. ~0x1), so >>> it may >>> - // be fine just to accept this as a warning. >>> - ExtVal |= llvm::APInt::getHighBitsSet(Width, Width - >>> Val.getBitWidth()); >>> - if (!(FlagMask & ~ExtVal)) >>> - return true; >>> - } >>> - >>> return false; >>> } >>> >>> @@ -14208,13 +14199,8 @@ void Sema::ActOnEnumBody(SourceLocation >>> } >>> } >>> >>> - FlagEnumAttr *FEAttr = Enum->getAttr<FlagEnumAttr>(); >>> - if (FEAttr) >>> - FEAttr->getFlagBits() = llvm::APInt(BestWidth, 0); >>> - >>> // Loop over all of the enumerator constants, changing their types to >>> match >>> - // the type of the enum if needed. If we have a flag type, we also >>> prepare the >>> - // FlagBits cache. >>> + // the type of the enum if needed. >>> for (auto *D : Elements) { >>> auto *ECD = cast_or_null<EnumConstantDecl>(D); >>> if (!ECD) continue; // Already issued a diagnostic. >>> @@ -14246,7 +14232,7 @@ void Sema::ActOnEnumBody(SourceLocation >>> // enum-specifier, each enumerator has the type of its >>> // enumeration. >>> ECD->setType(EnumType); >>> - goto flagbits; >>> + continue; >>> } else { >>> NewTy = BestType; >>> NewWidth = BestWidth; >>> @@ -14273,32 +14259,21 @@ void Sema::ActOnEnumBody(SourceLocation >>> ECD->setType(EnumType); >>> else >>> ECD->setType(NewTy); >>> - >>> -flagbits: >>> - // Check to see if we have a constant with exactly one bit set. Note >>> that x >>> - // & (x - 1) will be nonzero if and only if x has more than one bit >>> set. >>> - if (FEAttr) { >>> - llvm::APInt ExtVal = InitVal.zextOrSelf(BestWidth); >>> - if (ExtVal != 0 && !(ExtVal & (ExtVal - 1))) { >>> - FEAttr->getFlagBits() |= ExtVal; >>> - } >>> - } >>> } >>> >>> - if (FEAttr) { >>> + if (Enum->hasAttr<FlagEnumAttr>()) { >>> for (Decl *D : Elements) { >>> EnumConstantDecl *ECD = cast_or_null<EnumConstantDecl>(D); >>> if (!ECD) continue; // Already issued a diagnostic. >>> >>> llvm::APSInt InitVal = ECD->getInitVal(); >>> - if (InitVal != 0 && !IsValueInFlagEnum(Enum, InitVal, true)) >>> + if (InitVal != 0 && !InitVal.isPowerOf2() && >>> + !IsValueInFlagEnum(Enum, InitVal, true)) >>> Diag(ECD->getLocation(), diag::warn_flag_enum_constant_out_of_range) >>> << ECD << Enum; >>> } >>> } >>> >>> - >>> - >>> Enum->completeDefinition(BestType, BestPromotionType, >>> NumPositiveBits, NumNegativeBits); >>> >>> >>> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=246830&r1=246829&r2=246830&view=diff >>> >>> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=246830&r1=246829&r2=246830&view=diff> >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Sep 3 20:03:03 2015 >>> @@ -698,8 +698,6 @@ static bool ShouldDiagnoseSwitchCaseNotI >>> EnumValsTy::iterator &EI, >>> EnumValsTy::iterator &EIEnd, >>> const llvm::APSInt &Val) { >>> - bool FlagType = ED->hasAttr<FlagEnumAttr>(); >>> - >>> if (const DeclRefExpr *DRE = >>> dyn_cast<DeclRefExpr>(CaseExpr->IgnoreParenImpCasts())) { >>> if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) { >>> @@ -711,7 +709,7 @@ static bool ShouldDiagnoseSwitchCaseNotI >>> } >>> } >>> >>> - if (FlagType) { >>> + if (ED->hasAttr<FlagEnumAttr>()) { >>> return !S.IsValueInFlagEnum(ED, Val, false); >>> } else { >>> while (EI != EIEnd && EI->first < Val) >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >> >> >> > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits