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