patch: new attribute enable_if
Nick Lewycky
nicholas at mxc.ca
Fri Jan 10 12:43:10 PST 2014
Aaron Ballman wrote:
> This is fantastic, thank you for taking this on! Some minor comments
> and observations below.
Thank you for the thorough review! I haven't been able to fix everything
you pointed out, but I'm attaching an updated patch anyways, with
FIXME's in it.
The two issues unfixed are:
- Why does C++11 syntax think there's only one argument?
- Why is potential-constexpr evaluation failing on VarTemplateDecls?
>> Index: docs/LanguageExtensions.rst
>> ===================================================================
>> --- docs/LanguageExtensions.rst (revision 198923)
>> +++ docs/LanguageExtensions.rst (working copy)
>> @@ -1316,6 +1316,8 @@
>> Query the presence of this new mangling with
>> ``__has_feature(objc_protocol_qualifier_mangling)``.
>>
>> +.. _langext-overloading:
>> +
>> Function Overloading in C
>> =========================
>>
>> @@ -1398,6 +1400,84 @@
>>
>> Query for this feature with ``__has_extension(attribute_overloadable)``.
>>
>> +Controlling Overload Resolution
>> +===============================
>> +
>> +Clang introduces the ``enable_if`` attribute, which can be placed on function
>> +declarations to control which overload is selected based on the values of the
>> +function's arguments. When combined with the
>> +:ref:``overloadable<langext-overloading>`` attribute, this feature is also
>> +available in C.
>> +
>> +.. code-block:: c++
>> +
>> + int isdigit(int c);
>> + int isdigit(int c) __attribute__((enable_if(c>= -1&& c<= 255, "'c' must have the value of an unsigned char or EOF")));
>> +
>> + void foo(char c) {
>> + isdigit(c);
>> + isdigit(10);
>> + isdigit(-10); // results in a compile-time error.
>> + }
>> +
>> +The enable_if attribute takes two arguments, the first is an expression written
>> +in terms of the function parameters, the second is a string explaining why this
>> +overload candidate could not be selected to be displayed in diagnostics. The
>> +expression is part of the function signature for the purposes of determining
>> +whether it is a redeclaration (following the rules used when determining
>> +whether a C++ template specialization is ODR-equivalent), but is not part of
>> +the type.
>> +
>> +An enable_if expression will be evaluated by substituting the values of the
>> +parameters from the call site into the arguments in the expression and
>> +determining whether the result is true. If the result is false or could not be
>> +determined through constant expression evaluation, then this overload will not
>> +be chosen and the reason supplied in the string will be given to the user if
>> +their code does not compile as a result.
>> +
>> +Because the enable_if expression is an unevaluated context, there are no global
>> +state changes, nor the ability to pass information from the enable_if
>> +expression to the function body. As a worked example, suppose we want calls to
>
> "As a worked example" reads a bit funny to me, is it a typo?
Hm, it's a phrase I use to mean "an example that's been worked out to
show the reader how to do it" but that doesn't appear to be common
usage. Fixed to simply use "For example".
>> +strnlen(strbuf, maxlen) to resolve to strnlen_chk(strbuf, maxlen, size of
>> +strbuf) only if the size of strbuf can be determined:
>> +
>> +.. code-block:: c++
>> +
>> + __attribute__((always_inline))
>> + static inline size_t strnlen(const char *s, size_t maxlen)
>> + __attribute__((overloadable))
>> + __attribute__((enable_if(__builtin_object_size(s, 0) != -1))),
>> + "chosen when the buffer size is known but 'maxlen' is not")))
>> + {
>> + return strnlen_chk(s, maxlen, __builtin_object_size(s, 0));
>> + }
>> +
>> +Multiple enable_if attributes may be applied to a single declaration. In this
>> +case, the enable_if expressions are evaluated from left to right in the
>> +following manner. First the candidates whose enable_if expressions evaluate to
>
> "First" should be followed by a comma. Wow, that's nitpicky. ;-)
Done. :)
>> +false or cannot be evaluated are discarded. If the remaining candidates do not
>> +share ODR-equivalent enable_if expressions, the overload resolution is
>> +ambiguous. Otherwise, enable_if overload resolution continues with the next
>> +enable_if attribute on the candidates that have not been discarded and have
>> +remaining enable_if attributes. In this way, we pick the most specific
>> +overload out of a number of viable overloads using enable_if.
>> +
>> +.. code-block:: c++
>> + void f() __attribute__((enable_if(true, ""))); // #1
>> + void f() __attribute__((enable_if(true, ""))) __attribute__((enable_if(true, ""))); // #2
>> +
>> + void g(int i, int j) __attribute__((enable_if(i, ""))); // #1
>> + void g(int i, int j) __attribute__((enable_if(j, ""))) __attribute__((enable_if(true))); // #2
>> +
>> +In this example, a call to f() is always resolved to #2, as the first enable_if
>> +expression is ODR-equivalent for both declarations, but #1 does not have another
>> +enable_if expression to continue evaluating, so the next round of evaluation has
>> +only a single candidate. In a call to g(1, 1), the call is ambiguous even though
>> +#2 has more enable_if attributes, because the first enable_if expressions are
>> +not ODR-equivalent.
>> +
>> +Query for this feature with ``__has_attribute(enable_if)``.
>> +
>> Initializer lists for complex numbers in C
>> ==========================================
>>
>> Index: include/clang/AST/Expr.h
>> ===================================================================
>> --- include/clang/AST/Expr.h (revision 198923)
>> +++ include/clang/AST/Expr.h (working copy)
>> @@ -508,6 +508,16 @@
>> SmallVectorImpl<
>> PartialDiagnosticAt> &Diags);
>>
>> + /// isPotentialConstantExprUnevaluted - Return true if this expression might
>> + /// be usable in a constant expression in C++11 in an unevaluated context, if
>> + /// it were in function FD marked constexpr. Return false if the function can
>> + /// never produce a constant expression, along with diagnostics describing
>> + /// why not.
>> + static bool isPotentialConstantExprUnevaluated(Expr *E,
>> + const FunctionDecl *FD,
>> + SmallVectorImpl<
>> + PartialDiagnosticAt> &Diags);
>> +
>> /// isConstantInitializer - Returns true if this expression can be emitted to
>> /// IR as a constant, and thus can be used as a constant initializer in C.
>> bool isConstantInitializer(ASTContext&Ctx, bool ForRef) const;
>> @@ -600,6 +610,14 @@
>> const VarDecl *VD,
>> SmallVectorImpl<PartialDiagnosticAt> &Notes) const;
>>
>> + /// EvaluateWithSubstitution - Evaluate an expression as if from the context
>> + /// of a call to the given function with the given arguments, inside an
>> + /// unevaluated context. Returns true if the expression could be folded to a
>> + /// constant.
>> + bool EvaluateWithSubstitution(APValue&Value, ASTContext&Ctx,
>> + FunctionDecl *Callee,
>> + llvm::ArrayRef<const Expr*> Args) const;
>> +
>> /// \brief Enumeration used to describe the kind of Null pointer constant
>> /// returned from \c isNullPointerConstant().
>> enum NullPointerConstantKind {
>> Index: include/clang/Basic/Attr.td
>> ===================================================================
>> --- include/clang/Basic/Attr.td (revision 198923)
>> +++ include/clang/Basic/Attr.td (working copy)
>> @@ -470,6 +470,13 @@
>> let Subjects = SubjectList<[Function]>;
>> }
>>
>> +def EnableIf : InheritableAttr {
>> + let Spellings = [GNU<"enable_if">];
>
> Since this is also supported in C++, could we have a CXX11<"clang",
> "enable_if"> spelling as well?
I tried adding that, but it doesn't seem work out of the box:
void f(int n) [[clang::enable_if((n > 5), "chosen when 'n' is greater
than five")]];
produces "error: expected expression" while:
[[clang::enable_if((n > 5), "chosen when 'n' is greater than five")]]
void f(int n);
produces "error: 'enable_if' attribute requires exactly 2 arguments"!
That's going to take some debugging. I've added a testcase for this
commented out with a FIXME on it.
>> + let Subjects = SubjectList<[Function]>;
>
> Do you want this to apply to FunctionTemplate and perhaps ObjCMethod as well?
To quote Richard Smith, "this [attribute] should apply to the
FunctionDecl within a FunctionTemplateDecl, not to the template itself,
right?" -
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130923/089332.html
I'm not sure about ObjCMethod. I'm leaning towards not supporting it
simply because I don't know enough objc to write any testcases. I don't
have any ideological opposition here.
>> + let Args = [ExprArgument<"Cond">, StringArgument<"Message">];
>> + let TemplateDependent = 1;
>
> You *may* have to set LateParsed = 1 here as well, as the expressions
> are template dependant. I'm not 100% certain though.
Uh oh. I musn't set LateParsed = 1 here, because we need to parse it
before we can decide whether it's a redeclaration. Late parsed is too late.
Unfortunately, enable_if on a templated function looks pretty broken,
see my comments at the end. At the moment, I don't think that LateParsed
= 0 is the problem, but until I finish debugging it I can't be sure.
>> +}
>> +
>> def ExtVectorType : Attr {
>> let Spellings = [GNU<"ext_vector_type">];
>> let Subjects = SubjectList<[TypedefName], ErrorDiag>;
>> Index: include/clang/Basic/DiagnosticSemaKinds.td
>> ===================================================================
>> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 198923)
>> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
>> @@ -1739,6 +1739,8 @@
>> def ext_constexpr_function_never_constant_expr : ExtWarn<
>> "constexpr %select{function|constructor}0 never produces a"
>> "constant expression">, InGroup<DiagGroup<"invalid-constexpr">>, DefaultError;
>> +def err_enable_if_never_constant_expr : Error<
>> + "enable_if attribute expression never produces a constant expression">;
>
> I'd prefer the diagnostic to have enable_if in single quotes (it's
> more consistent with other attribute diagnostics then).
Done.
>> def err_constexpr_body_no_return : Error<
>> "no return statement in constexpr function">;
>> def warn_cxx11_compat_constexpr_body_no_return : Warning<
>> @@ -2588,6 +2590,8 @@
>> "candidate template ignored: substitution failure%0%1">;
>> def note_ovl_candidate_disabled_by_enable_if : Note<
>> "candidate template ignored: disabled by %0%1">;
>> +def note_ovl_candidate_disabled_by_enable_if_attr : Note<
>> + "candidate disabled: %0">;
>> def note_ovl_candidate_failed_overload_resolution : Note<
>> "candidate template ignored: couldn't resolve reference to overloaded"
>> "function %0">;
>> Index: include/clang/Parse/Parser.h
>> ===================================================================
>> --- include/clang/Parse/Parser.h (revision 198923)
>> +++ include/clang/Parse/Parser.h (working copy)
>> @@ -1972,7 +1972,7 @@
>> if (Tok.is(tok::kw___attribute)) {
>> ParsedAttributes attrs(AttrFactory);
>> SourceLocation endLoc;
>> - ParseGNUAttributes(attrs,&endLoc, LateAttrs);
>> + ParseGNUAttributes(attrs,&endLoc, LateAttrs,&D);
>> D.takeAttributes(attrs, endLoc);
>> }
>> }
>> @@ -1984,14 +1984,16 @@
>> }
>> void ParseGNUAttributes(ParsedAttributes&attrs,
>> SourceLocation *endLoc = 0,
>> - LateParsedAttrList *LateAttrs = 0);
>> + LateParsedAttrList *LateAttrs = 0,
>> + Declarator *D = 0);
>> void ParseGNUAttributeArgs(IdentifierInfo *AttrName,
>> SourceLocation AttrNameLoc,
>> ParsedAttributes&Attrs,
>> SourceLocation *EndLoc,
>> IdentifierInfo *ScopeName,
>> SourceLocation ScopeLoc,
>> - AttributeList::Syntax Syntax);
>> + AttributeList::Syntax Syntax,
>> + Declarator *D);
>> IdentifierLoc *ParseIdentifierLoc();
>>
>> void MaybeParseCXX11Attributes(Declarator&D) {
>> Index: include/clang/Sema/Overload.h
>> ===================================================================
>> --- include/clang/Sema/Overload.h (revision 198923)
>> +++ include/clang/Sema/Overload.h (working copy)
>> @@ -579,7 +579,11 @@
>> /// (CUDA) This candidate was not viable because the callee
>> /// was not accessible from the caller's target (i.e. host->device,
>> /// global->host, device->host).
>> - ovl_fail_bad_target
>> + ovl_fail_bad_target,
>> +
>> + /// This candidate function was not viable because an enable_if
>> + /// attribute disabled it.
>> + ovl_fail_enable_if
>> };
>>
>> /// OverloadCandidate - A single candidate in an overload set (C++ 13.3).
>> Index: include/clang/Sema/Sema.h
>> ===================================================================
>> --- include/clang/Sema/Sema.h (revision 198923)
>> +++ include/clang/Sema/Sema.h (working copy)
>> @@ -101,6 +101,7 @@
>> class DependentDiagnostic;
>> class DesignatedInitExpr;
>> class Designation;
>> + class EnableIfAttr;
>> class EnumConstantDecl;
>> class Expr;
>> class ExtVectorType;
>> @@ -2200,6 +2201,11 @@
>> // identified by the expression Expr
>> void NoteAllOverloadCandidates(Expr* E, QualType DestType = QualType());
>>
>> + /// Check the enable_if expressions on the given function. Returns the first
>> + /// failing attribute, or NULL if they were all successful.
>> + EnableIfAttr *CheckEnableIf(FunctionDecl *Function, ArrayRef<Expr *> Args,
>> + bool MissingImplicitThis = false);
>> +
>
> A bit of const love here would not make me sad, but isn't strictly required.
I tried making using ArrayRef<const Expr *> here but ended up pulling a
very long string (or else using bad const_casts). It's not worth it.
>> // [PossiblyAFunctionType] --> [Return]
>> // NonFunctionType --> NonFunctionType
>> // R (A) --> R(A)
>> @@ -4776,6 +4782,7 @@
>> AttributeList *AttrList);
>> void ActOnFinishCXXMemberDecls();
>>
>> + void ActOnReenterCXXMethodParameter(Scope *S, ParmVarDecl *Param);
>> void ActOnReenterTemplateScope(Scope *S, Decl *Template);
>> void ActOnReenterDeclaratorTemplateScope(Scope *S, DeclaratorDecl *D);
>> void ActOnStartDelayedMemberDeclarations(Scope *S, Decl *Record);
>> Index: lib/AST/ExprConstant.cpp
>> ===================================================================
>> --- lib/AST/ExprConstant.cpp (revision 198923)
>> +++ lib/AST/ExprConstant.cpp (working copy)
>> @@ -474,13 +474,30 @@
>>
>> /// Evaluate in any way we know how. Don't worry about side-effects that
>> /// can't be modeled.
>> - EM_IgnoreSideEffects
>> + EM_IgnoreSideEffects,
>> +
>> + /// Evaluate as a constant expression. Stop if we find that the expression
>> + /// is not a constant expression. Some expressions can be retried in the
>> + /// optimizer if we don't constant fold them here, but in an unevaluated
>> + /// context we try to fold them immediately since the optimizer never
>> + /// gets a chance to look at it.
>> + EM_ConstantExpressionUnevaluated,
>> +
>> + /// Evaluate as a potential constant expression. Keep going if we hit a
>> + /// construct that we can't evaluate yet (because we don't yet know the
>> + /// value of something) but stop if we hit something that could never be
>> + /// a constant expression. Some expressions can be retried in the
>> + /// optimizer if we don't constant fold them here, but in an unevaluated
>> + /// context we try to fold them immediately since the optimizer never
>> + /// gets a chance to look at it.
>> + EM_PotentialConstantExpressionUnevaluated
>> } EvalMode;
>>
>> /// Are we checking whether the expression is a potential constant
>> /// expression?
>> bool checkingPotentialConstantExpression() const {
>> - return EvalMode == EM_PotentialConstantExpression;
>> + return EvalMode == EM_PotentialConstantExpression ||
>> + EvalMode == EM_PotentialConstantExpressionUnevaluated;
>> }
>>
>> /// Are we checking an expression for overflow?
>> @@ -573,6 +590,8 @@
>> // some later problem.
>> case EM_ConstantExpression:
>> case EM_PotentialConstantExpression:
>> + case EM_ConstantExpressionUnevaluated:
>> + case EM_PotentialConstantExpressionUnevaluated:
>> HasActiveDiagnostic = false;
>> return OptionalDiagnostic();
>> }
>> @@ -644,11 +663,13 @@
>> bool keepEvaluatingAfterSideEffect() {
>> switch (EvalMode) {
>> case EM_PotentialConstantExpression:
>> + case EM_PotentialConstantExpressionUnevaluated:
>> case EM_EvaluateForOverflow:
>> case EM_IgnoreSideEffects:
>> return true;
>>
>> case EM_ConstantExpression:
>> + case EM_ConstantExpressionUnevaluated:
>> case EM_ConstantFold:
>> return false;
>> }
>> @@ -670,10 +691,12 @@
>>
>> switch (EvalMode) {
>> case EM_PotentialConstantExpression:
>> + case EM_PotentialConstantExpressionUnevaluated:
>> case EM_EvaluateForOverflow:
>> return true;
>>
>> case EM_ConstantExpression:
>> + case EM_ConstantExpressionUnevaluated:
>> case EM_ConstantFold:
>> case EM_IgnoreSideEffects:
>> return false;
>> @@ -696,7 +719,9 @@
>> Info.EvalStatus.Diag->empty()&&
>> !Info.EvalStatus.HasSideEffects),
>> OldMode(Info.EvalMode) {
>> - if (Enabled&& Info.EvalMode == EvalInfo::EM_ConstantExpression)
>> + if (Enabled&&
>> + (Info.EvalMode == EvalInfo::EM_ConstantExpression ||
>> + Info.EvalMode == EvalInfo::EM_ConstantExpressionUnevaluated))
>> Info.EvalMode = EvalInfo::EM_ConstantFold;
>> }
>> void keepDiagnostics() { Enabled = false; }
>> @@ -5985,7 +6010,17 @@
>>
>> // Expression had no side effects, but we couldn't statically determine the
>> // size of the referenced object.
>> - return Error(E);
>> + switch (Info.EvalMode) {
>> + case EvalInfo::EM_ConstantExpression:
>> + case EvalInfo::EM_PotentialConstantExpression:
>> + case EvalInfo::EM_ConstantFold:
>> + case EvalInfo::EM_EvaluateForOverflow:
>> + case EvalInfo::EM_IgnoreSideEffects:
>> + return Error(E);
>> + case EvalInfo::EM_ConstantExpressionUnevaluated:
>> + case EvalInfo::EM_PotentialConstantExpressionUnevaluated:
>> + return Success(-1ULL, E);
>
> std::numeric_limits<unsigned long long>::max()? Not heavily tied to it.
The other two callers in the file also use "Success(-1ULL,". Let me know
if you want it anyways and I'll update them too.
>> + }
>> }
>>
>> case Builtin::BI__builtin_bswap16:
>> @@ -8656,6 +8691,28 @@
>> return IsConstExpr;
>> }
>>
>> +bool Expr::EvaluateWithSubstitution(APValue&Value, ASTContext&Ctx,
>> + FunctionDecl *Callee,
>> + llvm::ArrayRef<const Expr*> Args) const {
>> + Expr::EvalStatus Status;
>> + EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpressionUnevaluated);
>> +
>> + ArgVector ArgValues(Args.size());
>> + for (ArrayRef<const Expr*>::iterator I = Args.begin(), E = Args.end();
>> + I != E; ++I) {
>> + if (!Evaluate(ArgValues[I - Args.begin()], Info, *I))
>> + // If evaluation fails, throw away the argument entirely.
>> + ArgValues[I - Args.begin()] = APValue();
>> + if (Info.EvalStatus.HasSideEffects)
>> + return false;
>> + }
>> +
>> + // Build fake call to Callee.
>> + CallStackFrame Frame(Info, Callee->getLocation(), Callee, /*This*/0,
>> + ArgValues.data());
>> + return Evaluate(Value, Info, this)&& !Info.EvalStatus.HasSideEffects;
>> +}
>> +
>> bool Expr::isPotentialConstantExpr(const FunctionDecl *FD,
>> SmallVectorImpl<
>> PartialDiagnosticAt> &Diags) {
>> @@ -8696,3 +8753,26 @@
>>
>> return Diags.empty();
>> }
>> +
>> +bool Expr::isPotentialConstantExprUnevaluated(Expr *E,
>> + const FunctionDecl *FD,
>> + SmallVectorImpl<
>> + PartialDiagnosticAt> &Diags) {
>> + Expr::EvalStatus Status;
>> + Status.Diag =&Diags;
>> +
>> + EvalInfo Info(FD->getASTContext(), Status,
>> + EvalInfo::EM_PotentialConstantExpressionUnevaluated);
>> +
>> + // Fabricate a call stack frame to give the arguments a plausible cover story.
>> + ArrayRef<const Expr*> Args;
>> + ArgVector ArgValues(0);
>> + bool Success = EvaluateArgs(Args, ArgValues, Info);
>> + assert(Success&&
>> + "Failed to set up arguments for potential constant evaluation");
>> + CallStackFrame Frame(Info, SourceLocation(), FD, 0, ArgValues.data());
>> +
>> + APValue ResultScratch;
>> + Evaluate(ResultScratch, Info, E);
>> + return Diags.empty();
>> +}
>> Index: lib/Parse/ParseDecl.cpp
>> ===================================================================
>> --- lib/Parse/ParseDecl.cpp (revision 198923)
>> +++ lib/Parse/ParseDecl.cpp (working copy)
>> @@ -117,7 +117,8 @@
>> /// We follow the C++ model, but don't allow junk after the identifier.
>> void Parser::ParseGNUAttributes(ParsedAttributes&attrs,
>> SourceLocation *endLoc,
>> - LateParsedAttrList *LateAttrs) {
>> + LateParsedAttrList *LateAttrs,
>> + Declarator *D) {
>> assert(Tok.is(tok::kw___attribute)&& "Not a GNU attribute list!");
>>
>> while (Tok.is(tok::kw___attribute)) {
>> @@ -153,7 +154,7 @@
>> // Handle "parameterized" attributes
>> if (!LateAttrs || !isAttributeLateParsed(*AttrName)) {
>> ParseGNUAttributeArgs(AttrName, AttrNameLoc, attrs, endLoc, 0,
>> - SourceLocation(), AttributeList::AS_GNU);
>> + SourceLocation(), AttributeList::AS_GNU, D);
>> continue;
>> }
>>
>> @@ -258,7 +259,8 @@
>> SourceLocation *EndLoc,
>> IdentifierInfo *ScopeName,
>> SourceLocation ScopeLoc,
>> - AttributeList::Syntax Syntax) {
>> + AttributeList::Syntax Syntax,
>> + Declarator *D) {
>>
>> assert(Tok.is(tok::l_paren)&& "Attribute arg list not starting with '('");
>>
>> @@ -278,17 +280,33 @@
>> return;
>> }
>>
>> +
>
> Extra newline snuck in here.
Fixed.
>> // Type safety attributes have their own grammar.
>> if (AttrKind == AttributeList::AT_TypeTagForDatatype) {
>> ParseTypeTagForDatatypeAttribute(*AttrName, AttrNameLoc, Attrs, EndLoc);
>> return;
>> }
>> +
>> // Some attributes expect solely a type parameter.
>> if (attributeIsTypeArgAttr(*AttrName)) {
>> ParseAttributeWithTypeArg(*AttrName, AttrNameLoc, Attrs, EndLoc);
>> return;
>> }
>>
>> + // These may refer to the function arguments, but need to be parsed early to
>> + // participate in determining whether it's a redeclaration.
>> + llvm::OwningPtr<ParseScope> PrototypeScope;
>> + if (AttrName->isStr("enable_if")&& D&& D->isFunctionDeclarator()) {
>> + DeclaratorChunk::FunctionTypeInfo FTI = D->getFunctionTypeInfo();
>> + PrototypeScope.reset(new ParseScope(this, Scope::FunctionPrototypeScope |
>> + Scope::FunctionDeclarationScope |
>> + Scope::DeclScope));
>> + for (unsigned i = 0; i != FTI.NumArgs; ++i) {
>> + ParmVarDecl *Param = cast<ParmVarDecl>(FTI.ArgInfo[i].Param);
>> + Actions.ActOnReenterCXXMethodParameter(getCurScope(), Param);
>> + }
>> + }
>> +
>> // Ignore the left paren location for now.
>> ConsumeParen();
>>
>> @@ -1164,7 +1182,7 @@
>> Actions.ActOnReenterFunctionContext(Actions.CurScope, D);
>>
>> ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs,&endLoc,
>> - 0, SourceLocation(), AttributeList::AS_GNU);
>> + 0, SourceLocation(), AttributeList::AS_GNU, 0);
>>
>> if (HasFunScope) {
>> Actions.ActOnExitFunctionContext();
>> @@ -1177,7 +1195,7 @@
>> // If there are multiple decls, then the decl cannot be within the
>> // function scope.
>> ParseGNUAttributeArgs(&LA.AttrName, LA.AttrNameLoc, Attrs,&endLoc,
>> - 0, SourceLocation(), AttributeList::AS_GNU);
>> + 0, SourceLocation(), AttributeList::AS_GNU, 0);
>> }
>> } else {
>> Diag(Tok, diag::warn_attribute_no_decl)<< LA.AttrName.getName();
>> Index: lib/Parse/ParseDeclCXX.cpp
>> ===================================================================
>> --- lib/Parse/ParseDeclCXX.cpp (revision 198923)
>> +++ lib/Parse/ParseDeclCXX.cpp (working copy)
>> @@ -3259,7 +3259,7 @@
>> if (Tok.is(tok::l_paren)) {
>> if (ScopeName&& ScopeName->getName() == "gnu") {
>> ParseGNUAttributeArgs(AttrName, AttrLoc, attrs, endLoc,
>> - ScopeName, ScopeLoc, AttributeList::AS_CXX11);
>> + ScopeName, ScopeLoc, AttributeList::AS_CXX11, 0);
>> AttrParsed = true;
>> } else {
>> if (StandardAttr)
>> Index: lib/Sema/SemaDeclAttr.cpp
>> ===================================================================
>> --- lib/Sema/SemaDeclAttr.cpp (revision 198923)
>> +++ lib/Sema/SemaDeclAttr.cpp (working copy)
>> @@ -827,6 +827,30 @@
>> Attr.getAttributeSpellingListIndex()));
>> }
>>
>> +static void handleEnableIfAttr(Sema&S, Decl *D, const AttributeList&Attr) {
>> + Expr *Cond = Attr.getArgAsExpr(0);
>> + ExprResult Converted = S.PerformContextuallyConvertToBool(Cond);
>> + if (Converted.isInvalid())
>> + return;
>> +
>> + StringRef Msg;
>> + if (!S.checkStringLiteralArgumentAttr(Attr, 1, Msg))
>> + return;
>> +
>> + SmallVector<PartialDiagnosticAt, 8> Diags;
>> + if (!Expr::isPotentialConstantExprUnevaluated(Cond, cast<FunctionDecl>(D),
>> + Diags)) {
>> + S.Diag(Attr.getLoc(), diag::err_enable_if_never_constant_expr);
>> + for (int I = 0, N = Diags.size(); I != N; ++I)
>> + S.Diag(Diags[I].first, Diags[I].second);
>> + return;
>> + }
>> +
>> + D->addAttr(::new (S.Context)
>> + EnableIfAttr(Attr.getRange(), S.Context, Converted.take(), Msg,
>> + Attr.getAttributeSpellingListIndex()));
>> +}
>> +
>> static void handleConsumableAttr(Sema&S, Decl *D, const AttributeList&Attr) {
>> ConsumableAttr::ConsumedState DefaultState;
>>
>> @@ -3990,6 +4014,7 @@
>> handleAttrWithMessage<DeprecatedAttr>(S, D, Attr);
>> break;
>> case AttributeList::AT_Destructor: handleDestructorAttr (S, D, Attr); break;
>> + case AttributeList::AT_EnableIf: handleEnableIfAttr (S, D, Attr); break;
>> case AttributeList::AT_ExtVectorType:
>> handleExtVectorTypeAttr(S, scope, D, Attr);
>> break;
>> Index: lib/Sema/SemaDeclCXX.cpp
>> ===================================================================
>> --- lib/Sema/SemaDeclCXX.cpp (revision 198923)
>> +++ lib/Sema/SemaDeclCXX.cpp (working copy)
>> @@ -6079,6 +6079,18 @@
>> PopDeclContext();
>> }
>>
>> +/// This is used to implement the constant expression evaluation part of the
>> +/// attribute enable_if extension. There is nothing in standard C++ which would
>> +/// require reentering parameters.
>> +void Sema::ActOnReenterCXXMethodParameter(Scope *S, ParmVarDecl *Param) {
>> + if (!Param)
>> + return;
>> +
>> + S->AddDecl(Param);
>> + if (Param->getDeclName())
>> + IdResolver.AddDecl(Param);
>> +}
>> +
>> /// ActOnStartDelayedCXXMethodDeclaration - We have completed
>> /// parsing a top-level (non-nested) C++ class, and we are now
>> /// parsing those parts of the given Method declaration that could
>> Index: lib/Sema/SemaExpr.cpp
>> ===================================================================
>> --- lib/Sema/SemaExpr.cpp (revision 198923)
>> +++ lib/Sema/SemaExpr.cpp (working copy)
>> @@ -4474,6 +4474,21 @@
>> else if (isa<MemberExpr>(NakedFn))
>> NDecl = cast<MemberExpr>(NakedFn)->getMemberDecl();
>>
>> + if (FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(NDecl)) {
>> + if (FD->hasAttr<EnableIfAttr>()) {
>> + if (EnableIfAttr *Attr = CheckEnableIf(FD, ArgExprs, true)) {
>> + Diag(Fn->getLocStart(),
>> + isa<CXXMethodDecl>(FD) ?
>> + diag::err_ovl_no_viable_member_function_in_call :
>> + diag::err_ovl_no_viable_function_in_call)
>> +<< FD->getDeclName()<< FD->getSourceRange();
>
> No need to pass getDeclName -- the diagnostics engine already
> understands NamedDecls.
Cool. Fixed!
>> + Diag(FD->getLocation(),
>> + diag::note_ovl_candidate_disabled_by_enable_if_attr)
>> +<< Attr->getCond()->getSourceRange()<< Attr->getMessage();
>> + }
>> + }
>> + }
>> +
>> return BuildResolvedCallExpr(Fn, NDecl, LParenLoc, ArgExprs, RParenLoc,
>> ExecConfig, IsExecConfig);
>> }
>> Index: lib/Sema/SemaOverload.cpp
>> ===================================================================
>> --- lib/Sema/SemaOverload.cpp (revision 198923)
>> +++ lib/Sema/SemaOverload.cpp (working copy)
>> @@ -1008,8 +1008,8 @@
>> isa<FunctionNoProtoType>(NewQType.getTypePtr()))
>> return false;
>>
>> - const FunctionProtoType* OldType = cast<FunctionProtoType>(OldQType);
>> - const FunctionProtoType* NewType = cast<FunctionProtoType>(NewQType);
>> + const FunctionProtoType *OldType = cast<FunctionProtoType>(OldQType);
>> + const FunctionProtoType *NewType = cast<FunctionProtoType>(NewQType);
>>
>> // The signature of a function includes the types of its
>> // parameters (C++ 1.3.10), which includes the presence or absence
>> @@ -1085,6 +1085,22 @@
>> return true;
>> }
>>
>> + // enable_if attributes are an order-sensitive part of the signature.
>
> So you are okay with them going in reverse order here?
Yes. We care that the two of them have the same enable_if attrs in the
same order, but we can check that in any order.
>> + for (specific_attr_iterator<EnableIfAttr>
>> + NewI = New->specific_attr_begin<EnableIfAttr>(),
>> + NewE = New->specific_attr_end<EnableIfAttr>(),
>> + OldI = Old->specific_attr_begin<EnableIfAttr>(),
>> + OldE = Old->specific_attr_end<EnableIfAttr>();
>> + NewI != NewE || OldI != OldE; ++NewI, ++OldI) {
>> + if (NewI == NewE || OldI == OldE)
>> + return true;
>> + llvm::FoldingSetNodeID NewID, OldID;
>> + NewI->getCond()->Profile(NewID, Context, true);
>> + OldI->getCond()->Profile(OldID, Context, true);
>> + if (!(NewID == OldID))
>
> Is there an != operator for FoldingSetNodeID?
There isn't! It's on my obvious cleanups list to add one and use it here.
>> + return true;
>> + }
>> +
>> // The signatures match; this is not an overload.
>> return false;
>> }
>> @@ -5452,11 +5468,11 @@
>> Sema::AddOverloadCandidate(FunctionDecl *Function,
>> DeclAccessPair FoundDecl,
>> ArrayRef<Expr *> Args,
>> - OverloadCandidateSet& CandidateSet,
>> + OverloadCandidateSet&CandidateSet,
>> bool SuppressUserConversions,
>> bool PartialOverloading,
>> bool AllowExplicit) {
>> - const FunctionProtoType* Proto
>> + const FunctionProtoType *Proto
>> = dyn_cast<FunctionProtoType>(Function->getType()->getAs<FunctionType>());
>> assert(Proto&& "Functions without a prototype cannot be overloaded");
>> assert(!Function->getDescribedFunctionTemplate()&&
>> @@ -5568,7 +5584,7 @@
>> if (Candidate.Conversions[ArgIdx].isBad()) {
>> Candidate.Viable = false;
>> Candidate.FailureKind = ovl_fail_bad_conversion;
>> - break;
>> + return;
>> }
>> } else {
>> // (C++ 13.3.2p2): For the purposes of overload resolution, any
>> @@ -5577,8 +5593,78 @@
>> Candidate.Conversions[ArgIdx].setEllipsis();
>> }
>> }
>> +
>> + if (EnableIfAttr *FailedAttr = CheckEnableIf(Function, Args)) {
>> + Candidate.Viable = false;
>> + Candidate.FailureKind = ovl_fail_enable_if;
>> + Candidate.DeductionFailure.Data = FailedAttr;
>> + return;
>> + }
>> }
>>
>> +static bool IsNotEnableIfAttr(Attr *A) { return !isa<EnableIfAttr>(A); }
>> +
>> +EnableIfAttr *Sema::CheckEnableIf(FunctionDecl *Function, ArrayRef<Expr *> Args,
>> + bool MissingImplicitThis) {
>> + // FIXME: specific_attr_iterator<EnableIfAttr> iterates in reverse order, but
>> + // we need to find the first failing one.
>> + if (!Function->hasAttrs()) return 0;
>
> Return should be on a new line.
Done.
>> + AttrVec Attrs = Function->getAttrs();
>> + AttrVec::iterator E = std::remove_if(Attrs.begin(), Attrs.end(),
>> + IsNotEnableIfAttr);
>> + if (Attrs.begin() == E) return 0;
>
> I think it would be more clear if it were Attrs.empty().
Nope! remove_if doesn't remove elements from the container, it shuffles
them to the end and returns a new 'end' iterator marking the point at
which elements fail the predicate.
Also a new line here.
Done.
>
>> + std::reverse(Attrs.begin(), E);
>> +
>> + SFINAETrap Trap(*this);
>> +
>> + // Convert the arguments.
>> + SmallVector<Expr *, 16> ConvertedArgs;
>> + bool InitializationFailed = false;
>> + for (unsigned i = 0, e = Args.size(); i != e; ++i) {
>> + if (i == 0&& !MissingImplicitThis&& isa<CXXMethodDecl>(Function)&&
>> + !cast<CXXMethodDecl>(Function)->isStatic()) {
>> + CXXMethodDecl *Method = cast<CXXMethodDecl>(Function);
>> + ExprResult R =
>> + PerformObjectArgumentInitialization(Args[0], /*Qualifier=*/0,
>> + Method, Method);
>> + if (R.isInvalid()) {
>> + InitializationFailed = true;
>> + break;
>> + }
>> + ConvertedArgs.push_back(R.take());
>> + } else {
>> + ExprResult R =
>> + PerformCopyInitialization(InitializedEntity::InitializeParameter(
>> + Context,
>> + Function->getParamDecl(i)),
>> + SourceLocation(),
>> + Args[i]);
>> + if (R.isInvalid()) {
>> + InitializationFailed = true;
>> + break;
>> + }
>> + ConvertedArgs.push_back(R.take());
>> + }
>> + }
>> +
>> + if (InitializationFailed || Trap.hasErrorOccurred()) {
>> + return cast<EnableIfAttr>(Attrs[0]);
>> + }
>
> Braces are not needed here.
Done.
>> +
>> + for (AttrVec::iterator I = Attrs.begin(); I != E; ++I) {
>> + APValue Result;
>> + EnableIfAttr *EIA = cast<EnableIfAttr>(*I);
>> + if (!EIA->getCond()->EvaluateWithSubstitution(
>> + Result, Context, Function,
>> + llvm::ArrayRef<const Expr*>(ConvertedArgs.data(),
>> + ConvertedArgs.size())) ||
>> + !Result.isInt() || !Result.getInt().getBoolValue()) {
>> + return EIA;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> /// \brief Add all of the function declarations in the given function set to
>> /// the overload candidate set.
>> void Sema::AddFunctionCandidates(const UnresolvedSetImpl&Fns,
>> @@ -5658,9 +5744,9 @@
>> CXXRecordDecl *ActingContext, QualType ObjectType,
>> Expr::Classification ObjectClassification,
>> ArrayRef<Expr *> Args,
>> - OverloadCandidateSet& CandidateSet,
>> + OverloadCandidateSet&CandidateSet,
>> bool SuppressUserConversions) {
>> - const FunctionProtoType* Proto
>> + const FunctionProtoType *Proto
>> = dyn_cast<FunctionProtoType>(Method->getType()->getAs<FunctionType>());
>> assert(Proto&& "Methods without a prototype cannot be overloaded");
>> assert(!isa<CXXConstructorDecl>(Method)&&
>> @@ -5747,15 +5833,22 @@
>> if (Candidate.Conversions[ArgIdx + 1].isBad()) {
>> Candidate.Viable = false;
>> Candidate.FailureKind = ovl_fail_bad_conversion;
>> - break;
>> + return;
>> }
>> } else {
>> // (C++ 13.3.2p2): For the purposes of overload resolution, any
>> // argument for which there is no corresponding parameter is
>> - // considered to ""match the ellipsis" (C+ 13.3.3.1.3).
>> + // considered to "match the ellipsis" (C+ 13.3.3.1.3).
>> Candidate.Conversions[ArgIdx + 1].setEllipsis();
>> }
>> }
>> +
>> + if (EnableIfAttr *FailedAttr = CheckEnableIf(Method, Args, true)) {
>> + Candidate.Viable = false;
>> + Candidate.FailureKind = ovl_fail_enable_if;
>> + Candidate.DeductionFailure.Data = FailedAttr;
>> + return;
>> + }
>> }
>>
>> /// \brief Add a C++ member function template as a candidate to the candidate
>> @@ -5971,7 +6064,7 @@
>> return;
>> }
>>
>> - // We won't go through a user-define type conversion function to convert a
>> + // We won't go through a user-defined type conversion function to convert a
>> // derived to base as such conversions are given Conversion Rank. They only
>> // go through a copy constructor. 13.3.3.1.2-p4 [over.ics.user]
>> QualType FromCanon
>> @@ -6031,6 +6124,7 @@
>> GetConversionRank(ICS.Standard.Second) != ICR_Exact_Match) {
>> Candidate.Viable = false;
>> Candidate.FailureKind = ovl_fail_final_conversion_not_exact;
>> + return;
>> }
>>
>> // C++0x [dcl.init.ref]p5:
>> @@ -6042,18 +6136,26 @@
>> ICS.Standard.First == ICK_Lvalue_To_Rvalue) {
>> Candidate.Viable = false;
>> Candidate.FailureKind = ovl_fail_bad_final_conversion;
>> + return;
>> }
>> break;
>>
>> case ImplicitConversionSequence::BadConversion:
>> Candidate.Viable = false;
>> Candidate.FailureKind = ovl_fail_bad_final_conversion;
>> - break;
>> + return;
>>
>> default:
>> llvm_unreachable(
>> "Can only end up with a standard conversion sequence or failure");
>> }
>> +
>> + if (EnableIfAttr *FailedAttr = CheckEnableIf(Conversion, ArrayRef<Expr*>())) {
>> + Candidate.Viable = false;
>> + Candidate.FailureKind = ovl_fail_enable_if;
>> + Candidate.DeductionFailure.Data = FailedAttr;
>> + return;
>> + }
>> }
>>
>> /// \brief Adds a conversion function template specialization
>> @@ -6191,7 +6293,7 @@
>> if (Candidate.Conversions[ArgIdx + 1].isBad()) {
>> Candidate.Viable = false;
>> Candidate.FailureKind = ovl_fail_bad_conversion;
>> - break;
>> + return;
>> }
>> } else {
>> // (C++ 13.3.2p2): For the purposes of overload resolution, any
>> @@ -6200,6 +6302,13 @@
>> Candidate.Conversions[ArgIdx + 1].setEllipsis();
>> }
>> }
>> +
>> + if (EnableIfAttr *FailedAttr = CheckEnableIf(Conversion, ArrayRef<Expr*>())) {
>> + Candidate.Viable = false;
>> + Candidate.FailureKind = ovl_fail_enable_if;
>> + Candidate.DeductionFailure.Data = FailedAttr;
>> + return;
>> + }
>> }
>>
>> /// \brief Add overload candidates for overloaded operators that are
>> @@ -8111,6 +8220,41 @@
>> }
>> }
>>
>> + // Check for enable_if value-based overload resolution.
>> + if (Cand1.Function&& Cand2.Function&&
>> + (Cand1.Function->hasAttr<EnableIfAttr>() ||
>> + Cand2.Function->hasAttr<EnableIfAttr>())) {
>> + // FIXME: The next several lines are just
>> + // specific_attr_iterator<EnableIfAttr> but going in declaration order,
>> + // instead of reverse order which is how they're stored in the AST.
>> + AttrVec Cand1Attrs = Cand1.Function->getAttrs();
>> + AttrVec::iterator Cand1E = std::remove_if(Cand1Attrs.begin(),
>> + Cand1Attrs.end(),
>> + IsNotEnableIfAttr);
>> + std::reverse(Cand1Attrs.begin(), Cand1E);
>> +
>> + AttrVec Cand2Attrs = Cand2.Function->getAttrs();
>> + AttrVec::iterator Cand2E = std::remove_if(Cand2Attrs.begin(),
>> + Cand2Attrs.end(),
>> + IsNotEnableIfAttr);
>> + std::reverse(Cand2Attrs.begin(), Cand2E);
>> + for (AttrVec::iterator
>> + Cand1I = Cand1Attrs.begin(), Cand2I = Cand2Attrs.begin();
>> + Cand1I != Cand1E || Cand2I != Cand2E; ++Cand1I, ++Cand2I) {
>> + if (Cand1I == Cand1E)
>> + return false;
>> + if (Cand2I == Cand2E)
>> + return true;
>> + llvm::FoldingSetNodeID Cand1ID, Cand2ID;
>> + cast<EnableIfAttr>(*Cand1I)->getCond()->Profile(Cand1ID,
>> + S.getASTContext(), true);
>> + cast<EnableIfAttr>(*Cand2I)->getCond()->Profile(Cand2ID,
>> + S.getASTContext(), true);
>> + if (!(Cand1ID == Cand2ID))
>
> operator != ?
Nope.
>> + return false;
>> + }
>> + }
>> +
>> return false;
>> }
>>
>> @@ -8819,6 +8963,15 @@
>> << (unsigned) FnKind<< CalleeTarget<< CallerTarget;
>> }
>>
>> +void DiagnoseFailedEnableIfAttr(Sema&S, OverloadCandidate *Cand) {
>> + FunctionDecl *Callee = Cand->Function;
>> + EnableIfAttr *Attr = static_cast<EnableIfAttr*>(Cand->DeductionFailure.Data);
>> +
>> + S.Diag(Callee->getLocation(),
>> + diag::note_ovl_candidate_disabled_by_enable_if_attr)
>> +<< Attr->getCond()->getSourceRange()<< Attr->getMessage();
>> +}
>> +
>> /// Generates a 'note' diagnostic for an overload candidate. We've
>> /// already generated a primary error at the call site.
>> ///
>> @@ -8882,6 +9035,9 @@
>>
>> case ovl_fail_bad_target:
>> return DiagnoseBadTarget(S, Cand);
>> +
>> + case ovl_fail_enable_if:
>> + return DiagnoseFailedEnableIfAttr(S, Cand);
>> }
>> }
>>
>> @@ -11107,7 +11263,7 @@
>> << qualsString
>> << (qualsString.find(' ') == std::string::npos ? 1 : 2);
>> }
>> -
>> +
>> CXXMemberCallExpr *call
>> = new (Context) CXXMemberCallExpr(Context, MemExprE, Args,
>> resultType, valueKind, RParenLoc);
>> Index: test/Sema/enable_if.c
>> ===================================================================
>> --- test/Sema/enable_if.c (revision 0)
>> +++ test/Sema/enable_if.c (working copy)
>> @@ -0,0 +1,97 @@
>> +// RUN: %clang_cc1 %s -verify -Wno-gcc-compat
>> +// RUN: %clang_cc1 %s -DCODEGEN -emit-llvm -o - -Wno-gcc-compat | FileCheck %s
>> +
>> +#define O_CREAT 0x100
>> +typedef int mode_t;
>> +typedef unsigned long size_t;
>> +
>> +int open(const char *pathname, int flags) __attribute__((enable_if(!(flags& O_CREAT), "must specify mode when using O_CREAT"))) __attribute__((overloadable)); // expected-note{{candidate disabled: must specify mode when using O_CREAT}}
>> +int open(const char *pathname, int flags, mode_t mode) __attribute__((overloadable)); // expected-note{{candidate function not viable: requires 3 arguments, but 2 were provided}}
>> +
>> +void test1() {
>> +#ifndef CODEGEN
>> + open("path", O_CREAT); // expected-error{{no matching function for call to 'open'}}
>> +#endif
>> + open("path", O_CREAT, 0660);
>> + open("path", 0);
>> + open("path", 0, 0);
>> +}
>> +
>> +size_t __strnlen_chk(const char *s, size_t requested_amount, size_t s_len);
>> +
>> +size_t strnlen(const char *s, size_t maxlen) // expected-note{{candidate function}}
>> + __attribute__((overloadable))
>> + __asm__("strnlen_real1");
>> +
>> +__attribute__((always_inline))
>> +inline size_t strnlen(const char *s, size_t maxlen) // expected-note{{candidate function}}
>> + __attribute__((overloadable))
>> + __attribute__((enable_if(__builtin_object_size(s, 0) != -1,
>> + "chosen when target buffer size is known")))
>> +{
>> + return __strnlen_chk(s, maxlen, __builtin_object_size(s, 0));
>> +}
>> +
>> +size_t strnlen(const char *s, size_t maxlen) // expected-note{{candidate disabled: chosen when 'maxlen' is known to be less than or equal to the buffer size}}
>> + __attribute__((overloadable))
>> + __attribute__((enable_if(__builtin_object_size(s, 0) != -1,
>> + "chosen when target buffer size is known")))
>> + __attribute__((enable_if(maxlen<= __builtin_object_size(s, 0),
>> + "chosen when 'maxlen' is known to be less than or equal to the buffer size")))
>> + __asm__("strnlen_real2");
>> +
>> +size_t strnlen(const char *s, size_t maxlen) // expected-note{{candidate function has been explicitly made unavailable}}
>> + __attribute__((overloadable))
>> + __attribute__((enable_if(__builtin_object_size(s, 0) != -1,
>> + "chosen when target buffer size is known")))
>> + __attribute__((enable_if(maxlen> __builtin_object_size(s, 0),
>> + "chosen when 'maxlen' is larger than the buffer size")))
>> + __attribute__((unavailable("'maxlen' is larger than the buffer size")));
>> +
>> +void test2(const char *s, int i) {
>> +// CHECK: define void @test2
>> + const char c[123];
>> + strnlen(s, i);
>> +// CHECK: call {{.*}}strnlen_real1
>> + strnlen(s, 999);
>> +// CHECK: call {{.*}}strnlen_real1
>> + strnlen(c, 1);
>> +// CHECK: call {{.*}}strnlen_real2
>> + strnlen(c, i);
>> +// CHECK: call {{.*}}strnlen_chk
>> +#ifndef CODEGEN
>> + strnlen(c, 999); // expected-error{{call to unavailable function 'strnlen': 'maxlen' is larger than the buffer size}}
>> +#endif
>> +}
>> +
>> +int isdigit(int c) __attribute__((overloadable)); // expected-note{{candidate function}}
>> +int isdigit(int c) __attribute__((overloadable)) // expected-note{{candidate function has been explicitly made unavailable}}
>> + __attribute__((enable_if(c<= -1 || c> 255, "'c' must have the value of an unsigned char or EOF")))
>> + __attribute__((unavailable("'c' must have the value of an unsigned char or EOF")));
>> +
>> +void test3(int c) {
>> + isdigit(c);
>> + isdigit(10);
>> +#ifndef CODEGEN
>> + isdigit(-10); // expected-error{{call to unavailable function 'isdigit': 'c' must have the value of an unsigned char or EOF}}
>> +#endif
>> +}
>> +
>> +#ifndef CODEGEN
>> +__attribute__((enable_if(n == 0, "chosen when 'n' is zero"))) void f1(int n); // expected-error{{use of undeclared identifier 'n'}}
>> +
>> +int n __attribute__((enable_if(1, "always chosen"))); // expected-warning{{'enable_if' attribute only applies to functions}}
>> +
>> +void f(int n) __attribute__((enable_if("chosen when 'n' is zero", n == 0))); // expected-error{{'enable_if' attribute requires a string}}
>> +
>> +void f(int n) __attribute__((enable_if())); // expected-error{{'enable_if' attribute requires exactly 2 arguments}}
>> +
>> +void f(int n) __attribute__((enable_if(unresolvedid, "chosen when 'unresolvedid' is non-zero"))); // expected-error{{use of undeclared identifier 'unresolvedid'}}
>> +
>> +int global;
>> +void f(int n) __attribute__((enable_if(global == 0, "chosen when 'global' is zero"))); // expected-error{{enable_if attribute expression never produces a constant expression}} // expected-note{{subexpression not valid in a constant expression}}
>> +
>> +const int cst = 7;
>> +void return_cst(void) __attribute__((overloadable)) __attribute__((enable_if(cst == 7, "chosen when 'cst' is 7")));
>> +void test_return_cst() { return_cst(); }
>> +#endif
>> Index: test/SemaCXX/enable_if.cpp
>> ===================================================================
>> --- test/SemaCXX/enable_if.cpp (revision 0)
>> +++ test/SemaCXX/enable_if.cpp (working copy)
>> @@ -0,0 +1,56 @@
>> +// RUN: %clang_cc1 -std=c++11 -verify -Wno-gcc-compat %s
>> +
>> +typedef int (*fp)(int);
>> +int surrogate(int);
>> +
>> +struct X {
>> + void f(int n) __attribute__((enable_if(n == 0, "chosen when 'n' is zero")));
>> + void f(int n) __attribute__((enable_if(n == 1, "chosen when 'n' is one"))); // expected-note{{member declaration nearly matches}} expected-note{{candidate disabled: chosen when 'n' is one}}
>> +
>> + static void s(int n) __attribute__((enable_if(n == 0, "chosen when 'n' is zero"))); // expected-note2{{candidate disabled: chosen when 'n' is zero}}
>> +
>> + void conflict(int n) __attribute__((enable_if(n+n == 10, "chosen when 'n' is five"))); // expected-note{{candidate function}}
>> + void conflict(int n) __attribute__((enable_if(n*2 == 10, "chosen when 'n' is five"))); // expected-note{{candidate function}}
>> +
>> + operator long() __attribute__((enable_if(true, "chosen on your platform")));
>> + operator int() __attribute__((enable_if(false, "chosen on other platform")));
>> +
>> + operator fp() __attribute__((enable_if(false, "never enabled"))) { return surrogate; } // expected-note{{conversion candidate of type 'int (*)(int)'}} // FIXME: the message is not displayed
>> +};
>> +
>> +void X::f(int n) __attribute__((enable_if(n == 0, "chosen when 'n' is zero"))) // expected-note{{member declaration nearly matches}} expected-note{{candidate disabled: chosen when 'n' is zero}}
>> +{
>> +}
>> +
>> +void X::f(int n) __attribute__((enable_if(n == 2, "chosen when 'n' is two"))) // expected-error{{out-of-line definition of 'f' does not match any declaration in 'X'}} expected-note{{candidate disabled: chosen when 'n' is two}}
>> +{
>> +}
>> +
>> +__attribute__((deprecated)) constexpr int old() { return 0; } // expected-note2{{'old' has been explicitly marked deprecated here}}
>> +void deprec1(int i) __attribute__((enable_if(old() == 0, "chosen when old() is zero"))); // expected-warning{{'old' is deprecated}}
>> +void deprec2(int i) __attribute__((enable_if(old() == 0, "chosen when old() is zero"))); // expected-warning{{'old' is deprecated}}
>> +
>> +void overloaded(int);
>> +void overloaded(long);
>> +
>> +void test() {
>> + X x;
>> + x.f(0);
>> + x.f(1);
>> + x.f(2); // no error, suppressed by erroneous out-of-line definition
>> + x.f(3); // expected-error{{no matching member function for call to 'f'}}
>> +
>> + x.s(0);
>> + x.s(1); // expected-error{{no matching member function for call to 's'}}
>> +
>> + X::s(0);
>> + X::s(1); // expected-error{{no matching member function for call to 's'}}
>> +
>> + x.conflict(5); // expected-error{{call to member function 'conflict' is ambiguous}}
>> +
>> + deprec2(0);
>> +
>> + overloaded(x);
>> +
>> + int i = x(1); // expected-error{{no matching function for call to object of type 'X'}}
>> +}
>
> Some tests involving templates might be kind of interesting.
Agreed! I was just wondering whether:
template<int N> void foo() __attribute__((enable_if(N == 1)));
template<int N> void foo() __attribute__((enable_if(N == 1)));
is considered one declaration + redeclaration, or two declarations. The
verdict? "'enable_if' attribute expression never produces a constant". Sigh.
This also requires further debugging. The good news is that it is aware
that 'N' is a valid identifier, which suggests it isn't a parsing
problem. I expect this to just be a problem to be fixed in potential
constant expression evaluation -- I'll see whether I can reproduce it
with a templated constexpr function.
Nick
> ~Aaron
>
> On Fri, Jan 10, 2014 at 6:11 AM, Nick Lewycky<nicholas at mxc.ca> wrote:
>> Richard Smith wrote:
>>>
>>> This looks really good. Just a few things:
>>>
>>>
>>> In LanguageExtensions.rst:
>>>
>>> "In this way, we pick the more specific"
>>>
>>> ... missing end of sentence.
>>
>>
>> Fixed.
>>
>>
>>> SemaDeclAttr.cpp: can the Subject and number-of-arguments checking be
>>> handled by the common attribute-handling code?
>>
>>
>> Turns out it can! Done.
>>
>>
>>> CheckEnableIf: can you perform the parameter initialization/conversion
>>> steps once, rather than once per enable_if attribute? Also, perhaps use
>>> std::remove_if rather than the current O(n^2) loop to extract the
>>> enable_if attributes (here and later in the same file), or maybe
>>> directly iterate over the attribute lists without the additional
>>> copy/filter?
>>
>>
>> Done with remove_if + reverse. It's an improvement.
>>
>> One other change, I updated the docs to not use __builtin_constant_p either.
>> This:
>> void foo(int i) __attribute__((enable_if(__builtin_constant_p(i))));
>> ...
>> foo(expr)
>> does not actually evaluate __builtin_constant_p(expr).
>>
>> Updated patch attached.
>>
>> Nick
>>
>>> On Mon, Dec 30, 2013 at 8:27 PM, Nick Lewycky<nicholas at mxc.ca
>>> <mailto:nicholas at mxc.ca>> wrote:
>>>
>>> This is the continuation of this thread:
>>>
>>> http://lists.cs.uiuc.edu/ pipermail/cfe-commits/Week-of-
>>> Mon-20131007/090357.html
>>>
>>> <http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131007/090357.html>
>>>
>>> from October 7.
>>>
>>> This update to the patch includes a significant change in semantics.
>>> We now support partial ordering of candidates when more than one
>>> enable_if attribute is applied to a declaration. The exact semantics
>>> of this feature are described in the LanguageExtensions.rst portion
>>> of the patch, instead I want to talk about the rationale.
>>>
>>> With the previous form of enable_if, overload resolution would be
>>> ambiguous unless all but one expression folded to false. Suppose you
>>> have four properties to test:
>>> p1) nothing is known, call regular strnlen
>>> p2) array length is known and maxlen isn't known, call strnlen_chk
>>> p3) array length is known and maxlen is known and it fits
>>> p4) array length is known and maxlen is known and it doesn't fit
>>>
>>> The four function declarations would need to each test all four of
>>> those properties. That's a little ugly, but there's one gotcha that
>>> makes it awful: an enable_if expression is a tristate, it could be
>>> true, false or could-not-compute (which is observed as false). This
>>> means that simply writing __attribute__((enable_if(p1&& !p2&&
>>> ...))) doesn't work.
>>>
>>> Instead, we handled this case by making user call
>>> __builtin_constant_p() in their enable_if expression to check
>>> whether a given argument was a constant, but then that necessitated
>>> changes to BCP's constant folding, changes that may make BCP behave
>>> differently from GCC.
>>>
>>> The new approach is to add partial function ordering semantics based
>>> on the rules for partial ordering of function templates
>>> [temp.func.order]. It's the same as "function with largest number of
>>> enable_if attributes wins" except that if the n-th enable_if
>>> expression on two candidates are not ODR-equivalent, then the two
>>> are ambiguous regardless of who has more enable_if attributes.
>>>
>>> This allows us to implement our four property case with:
>>> i1) a base case with no enable_if attributes
>>> i2) enable_if(__builtin_object_ size(s) != -1)
>>> i3) enable_if(__builtin_object_ size(s) != -1) enable_if(maxlen
>>> <= __builtin_object_size(s))
>>> i4) enable_if(__builtin_object_ size(s) != -1) enable_if(maxlen>
>>>
>>> __builtin_object_size(s))
>>>
>>> In particular, all the tests from the previous version of this patch
>>> have been updated to use multiple enable_if attributes where
>>> necessary, and none of them use __builtin_constant_p any more.
>>> Victory!
>>>
>>> Well, let's not declare victory yet. The implementation has some
>>> truly nasty stuff as seen in the patch. The major one is that I
>>> can't use specific_attr_iterator in two places I want to because it
>>> doesn't guarantee the correct ordering. The replacement code is akin
>>> to a lovecraftian horror. Yes, I want to commit it like this first
>>> and fix it later.
>>>
>>> I'm also not completely confident in my changes to ExprConstant.cpp.
>>> Does adding new EvalModes make sense for this? What is the
>>> difference between EM_ConstantExpression and EM_ConstantFold, and
>>> could I have made use of that instead of adding new modes?
>>>
>>> Please review. I'd appreciate making note of which review comments
>>> block submission, and which can be incremental improvements after
>>> the initial commit.
>>>
>>> Nick
>>>
>>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: enable_if-6.patch
Type: text/x-diff
Size: 45782 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140110/67b04a5f/attachment.patch>
More information about the cfe-commits
mailing list