<div dir="ltr">Jorge,<div><br></div><div>Why did you revert this?</div><div><br></div><div>/Eric</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Apr 27, 2019 at 6:01 AM Roman Lebedev via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sat, Apr 27, 2019 at 3:29 AM Jorge Gorbe Moya via cfe-commits<br>
<<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
><br>
> Author: jgorbe<br>
> Date: Fri Apr 26 17:32:04 2019<br>
> New Revision: 359361<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=359361&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=359361&view=rev</a><br>
> Log:<br>
> Revert Fix interactions between __builtin_constant_p and constexpr to match current trunk GCC.<br>
><br>
> This reverts r359059 (git commit 0b098754b73f3b96d00ecb1c7605760b11c90298)<br>
It is common to specify the *reason* for the revert, so it is recorded<br>
in change log.<br>
<br>
> Removed:<br>
> cfe/trunk/test/SemaCXX/builtin-constant-p.cpp<br>
> Modified:<br>
> cfe/trunk/lib/AST/ExprConstant.cpp<br>
> cfe/trunk/lib/Sema/SemaChecking.cpp<br>
> cfe/trunk/test/SemaCXX/enable_if.cpp<br>
><br>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359361&r1=359360&r2=359361&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359361&r1=359360&r2=359361&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)<br>
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Apr 26 17:32:04 2019<br>
> @@ -7801,33 +7801,19 @@ EvaluateBuiltinClassifyType(const CallEx<br>
> }<br>
><br>
> /// EvaluateBuiltinConstantPForLValue - Determine the result of<br>
> -/// __builtin_constant_p when applied to the given pointer.<br>
> +/// __builtin_constant_p when applied to the given lvalue.<br>
> ///<br>
> -/// A pointer is only "constant" if it is null (or a pointer cast to integer)<br>
> -/// or it points to the first character of a string literal.<br>
> -static bool EvaluateBuiltinConstantPForLValue(const APValue &LV) {<br>
> - APValue::LValueBase Base = LV.getLValueBase();<br>
> - if (Base.isNull()) {<br>
> - // A null base is acceptable.<br>
> - return true;<br>
> - } else if (const Expr *E = Base.dyn_cast<const Expr *>()) {<br>
> - if (!isa<StringLiteral>(E))<br>
> - return false;<br>
> - return LV.getLValueOffset().isZero();<br>
> - } else {<br>
> - // Any other base is not constant enough for GCC.<br>
> - return false;<br>
> - }<br>
> +/// An lvalue is only "constant" if it is a pointer or reference to the first<br>
> +/// character of a string literal.<br>
> +template<typename LValue><br>
> +static bool EvaluateBuiltinConstantPForLValue(const LValue &LV) {<br>
> + const Expr *E = LV.getLValueBase().template dyn_cast<const Expr*>();<br>
> + return E && isa<StringLiteral>(E) && LV.getLValueOffset().isZero();<br>
> }<br>
><br>
> /// EvaluateBuiltinConstantP - Evaluate __builtin_constant_p as similarly to<br>
> /// GCC as we can manage.<br>
> -static bool EvaluateBuiltinConstantP(EvalInfo &Info, const Expr *Arg) {<br>
> - // Constant-folding is always enabled for the operand of __builtin_constant_p<br>
> - // (even when the enclosing evaluation context otherwise requires a strict<br>
> - // language-specific constant expression).<br>
> - FoldConstant Fold(Info, true);<br>
> -<br>
> +static bool EvaluateBuiltinConstantP(ASTContext &Ctx, const Expr *Arg) {<br>
> QualType ArgType = Arg->getType();<br>
><br>
> // __builtin_constant_p always has one operand. The rules which gcc follows<br>
> @@ -7835,27 +7821,34 @@ static bool EvaluateBuiltinConstantP(Eva<br>
> //<br>
> // - If the operand is of integral, floating, complex or enumeration type,<br>
> // and can be folded to a known value of that type, it returns 1.<br>
> - // - If the operand can be folded to a pointer to the first character<br>
> - // of a string literal (or such a pointer cast to an integral type)<br>
> - // or to a null pointer or an integer cast to a pointer, it returns 1.<br>
> + // - If the operand and can be folded to a pointer to the first character<br>
> + // of a string literal (or such a pointer cast to an integral type), it<br>
> + // returns 1.<br>
> //<br>
> // Otherwise, it returns 0.<br>
> //<br>
> // FIXME: GCC also intends to return 1 for literals of aggregate types, but<br>
> // its support for this does not currently work.<br>
> - if (ArgType->isIntegralOrEnumerationType() || ArgType->isFloatingType() ||<br>
> - ArgType->isAnyComplexType() || ArgType->isPointerType() ||<br>
> - ArgType->isNullPtrType()) {<br>
> - APValue V;<br>
> - if (!::EvaluateAsRValue(Info, Arg, V))<br>
> + if (ArgType->isIntegralOrEnumerationType()) {<br>
> + Expr::EvalResult Result;<br>
> + if (!Arg->EvaluateAsRValue(Result, Ctx) || Result.HasSideEffects)<br>
> return false;<br>
><br>
> - // For a pointer (possibly cast to integer), there are special rules.<br>
> + APValue &V = Result.Val;<br>
> + if (V.getKind() == APValue::Int)<br>
> + return true;<br>
> if (V.getKind() == APValue::LValue)<br>
> return EvaluateBuiltinConstantPForLValue(V);<br>
> -<br>
> - // Otherwise, any constant value is good enough.<br>
> - return V.getKind() != APValue::Uninitialized;<br>
> + } else if (ArgType->isFloatingType() || ArgType->isAnyComplexType()) {<br>
> + return Arg->isEvaluatable(Ctx);<br>
> + } else if (ArgType->isPointerType() || Arg->isGLValue()) {<br>
> + LValue LV;<br>
> + Expr::EvalStatus Status;<br>
> + EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantFold);<br>
> + if ((Arg->isGLValue() ? EvaluateLValue(Arg, LV, Info)<br>
> + : EvaluatePointer(Arg, LV, Info)) &&<br>
> + !Status.HasSideEffects)<br>
> + return EvaluateBuiltinConstantPForLValue(LV);<br>
> }<br>
><br>
> // Anything else isn't considered to be sufficiently constant.<br>
> @@ -8266,7 +8259,7 @@ bool IntExprEvaluator::VisitBuiltinCallE<br>
><br>
> case Builtin::BI__builtin_constant_p: {<br>
> auto Arg = E->getArg(0);<br>
> - if (EvaluateBuiltinConstantP(Info, Arg))<br>
> + if (EvaluateBuiltinConstantP(Info.Ctx, Arg))<br>
> return Success(true, E);<br>
> auto ArgTy = Arg->IgnoreImplicit()->getType();<br>
> if (!Info.InConstantContext && !Arg->HasSideEffects(Info.Ctx) &&<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=359361&r1=359360&r2=359361&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=359361&r1=359360&r2=359361&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Apr 26 17:32:04 2019<br>
> @@ -1199,14 +1199,10 @@ Sema::CheckBuiltinFunctionCall(FunctionD<br>
> if (checkArgCount(*this, TheCall, 1)) return true;<br>
> TheCall->setType(Context.IntTy);<br>
> break;<br>
> - case Builtin::BI__builtin_constant_p: {<br>
> + case Builtin::BI__builtin_constant_p:<br>
> if (checkArgCount(*this, TheCall, 1)) return true;<br>
> - ExprResult Arg = DefaultFunctionArrayLvalueConversion(TheCall->getArg(0));<br>
> - if (Arg.isInvalid()) return true;<br>
> - TheCall->setArg(0, Arg.get());<br>
> TheCall->setType(Context.IntTy);<br>
> break;<br>
> - }<br>
> case Builtin::BI__builtin_launder:<br>
> return SemaBuiltinLaunder(*this, TheCall);<br>
> case Builtin::BI__sync_fetch_and_add:<br>
><br>
> Removed: cfe/trunk/test/SemaCXX/builtin-constant-p.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/builtin-constant-p.cpp?rev=359360&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/builtin-constant-p.cpp?rev=359360&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/builtin-constant-p.cpp (original)<br>
> +++ cfe/trunk/test/SemaCXX/builtin-constant-p.cpp (removed)<br>
> @@ -1,61 +0,0 @@<br>
> -// RUN: %clang_cc1 -std=c++17 -verify %s<br>
> -<br>
> -using intptr_t = __INTPTR_TYPE__;<br>
> -<br>
> -// Test interaction of constexpr and __builtin_constant_p.<br>
> -<br>
> -template<typename T> constexpr bool bcp(T t) {<br>
> - return __builtin_constant_p(t);<br>
> -}<br>
> -template<typename T> constexpr bool bcp_fold(T t) {<br>
> - return __builtin_constant_p(((void)(intptr_t)&t, t));<br>
> -}<br>
> -<br>
> -constexpr intptr_t ensure_fold_is_generally_not_enabled = // expected-error {{constant expression}}<br>
> - (intptr_t)&ensure_fold_is_generally_not_enabled; // expected-note {{cast}}<br>
> -<br>
> -constexpr intptr_t ptr_to_int(const void *p) {<br>
> - return __builtin_constant_p(1) ? (intptr_t)p : (intptr_t)p;<br>
> -}<br>
> -<br>
> -constexpr int *int_to_ptr(intptr_t n) {<br>
> - return __builtin_constant_p(1) ? (int*)n : (int*)n;<br>
> -}<br>
> -<br>
> -int x;<br>
> -<br>
> -// Integer and floating point constants encountered during constant expression<br>
> -// evaluation are considered constant. So is nullptr_t.<br>
> -static_assert(bcp(1));<br>
> -static_assert(bcp_fold(1));<br>
> -static_assert(bcp(1.0));<br>
> -static_assert(bcp_fold(1.0));<br>
> -static_assert(bcp(nullptr));<br>
> -static_assert(bcp_fold(nullptr));<br>
> -<br>
> -// Pointers to the start of strings are considered constant.<br>
> -static_assert(bcp("foo"));<br>
> -static_assert(bcp_fold("foo"));<br>
> -<br>
> -// Null pointers are considered constant.<br>
> -static_assert(bcp<int*>(nullptr));<br>
> -static_assert(bcp_fold<int*>(nullptr));<br>
> -static_assert(bcp<const char*>(nullptr));<br>
> -static_assert(bcp_fold<const char*>(nullptr));<br>
> -<br>
> -// Other pointers are not.<br>
> -static_assert(!bcp(&x));<br>
> -static_assert(!bcp_fold(&x));<br>
> -<br>
> -// Pointers cast to integers follow the rules for pointers.<br>
> -static_assert(bcp(ptr_to_int("foo")));<br>
> -static_assert(bcp_fold(ptr_to_int("foo")));<br>
> -static_assert(!bcp(ptr_to_int(&x)));<br>
> -static_assert(!bcp_fold(ptr_to_int(&x)));<br>
> -<br>
> -// Integers cast to pointers follow the integer rules.<br>
> -static_assert(bcp(int_to_ptr(0)));<br>
> -static_assert(bcp_fold(int_to_ptr(0)));<br>
> -static_assert(bcp(int_to_ptr(123))); // GCC rejects these due to not recognizing<br>
> -static_assert(bcp_fold(int_to_ptr(123))); // the bcp conditional in 'int_to_ptr' ...<br>
> -static_assert(__builtin_constant_p((int*)123)); // ... but GCC accepts this<br>
><br>
> Modified: cfe/trunk/test/SemaCXX/enable_if.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=359361&r1=359360&r2=359361&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=359361&r1=359360&r2=359361&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/enable_if.cpp (original)<br>
> +++ cfe/trunk/test/SemaCXX/enable_if.cpp Fri Apr 26 17:32:04 2019<br>
> @@ -522,14 +522,3 @@ void test() {<br>
> InConstantContext::foo("abc");<br>
> }<br>
> } // namespace InConstantContext<br>
> -<br>
> -namespace StringLiteralDetector {<br>
> - void need_string_literal(const char *p) __attribute__((enable_if(__builtin_constant_p(p), "argument is not a string literal"))); // expected-note 2{{not a string literal}}<br>
> - void test(const char *unknown) {<br>
> - need_string_literal("foo");<br>
> - need_string_literal(unknown); // expected-error {{no matching function}}<br>
> - constexpr char str[] = "bar";<br>
> - need_string_literal(str); // expected-error {{no matching function}}<br>
> - }<br>
> -}<br>
> -<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>