patch: new attribute enable_if
Aaron Ballman
aaron at aaronballman.com
Fri Jan 10 06:12:45 PST 2014
This is fantastic, thank you for taking this on! Some minor comments
and observations below.
> 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?
> +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. ;-)
> +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?
> + let Subjects = SubjectList<[Function]>;
Do you want this to apply to FunctionTemplate and perhaps ObjCMethod as well?
> + 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.
> +}
> +
> 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).
> 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.
> // [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.
> + }
> }
>
> 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.
> // 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.
> + 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?
> + 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?
> + 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.
> + 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(). Also a new line here.
> + 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.
> +
> + 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 != ?
> + 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.
~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
>
More information about the cfe-commits
mailing list