Re: r355743 - [8.0 Regression] Fix handling of `__builtin_constant_p` inside template arguments, enumerators, case statements, and the enable_if attribute.
Merged to release_80 in r355898.
On Fri, Mar 8, 2019 at 11:05 PM Eric Fiselier via cfe-commits
wrote:
>
> Author: ericwf
> Date: Fri Mar 8 14:06:48 2019
> New Revision: 355743
>
> URL: http://llvm.org/viewvc/llvm-project?rev=355743=rev
> Log:
> [8.0 Regression] Fix handling of `__builtin_constant_p` inside template
> arguments, enumerators, case statements, and the enable_if attribute.
>
> Summary:
> The following code is accepted by Clang 7 and prior but rejected by the
> upcoming 8 release and in trunk [1]
>
> ```
> // error {{never produces a constant expression}}
> void foo(const char* s) __attribute__((enable_if(__builtin_constant_p(*s) ==
> false, "trap"))) {}
> void test() { foo("abc"); }
> ```
>
> Prior to Clang 8, the call to `__builtin_constant_p` was a constant
> expression returning false. Currently, it's not a valid constant expression.
>
> The bug is caused because we failed to set `InConstantContext` when
> attempting to evaluate unevaluated constant expressions.
>
> [1] https://godbolt.org/z/ksAjmq
>
> Reviewers: rsmith, hans, sbenza
>
> Reviewed By: rsmith
>
> Subscribers: kristina, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D59038
>
> Modified:
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp
> cfe/trunk/test/SemaCXX/enable_if.cpp
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=355743=355742=355743=diff
> ==
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Mar 8 14:06:48 2019
> @@ -11131,6 +11131,7 @@ bool Expr::EvaluateAsConstantExpr(EvalRe
>const ASTContext ) const {
>EvalInfo::EvaluationMode EM = EvalInfo::EM_ConstantExpression;
>EvalInfo Info(Ctx, Result, EM);
> + Info.InConstantContext = true;
>if (!::Evaluate(Result.Val, Info, this))
> return false;
>
> @@ -11771,6 +11772,7 @@ bool Expr::EvaluateWithSubstitution(APVa
> const Expr *This) const {
>Expr::EvalStatus Status;
>EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpressionUnevaluated);
> + Info.InConstantContext = true;
>
>LValue ThisVal;
>const LValue *ThisPtr = nullptr;
> @@ -11854,6 +11856,7 @@ bool Expr::isPotentialConstantExprUneval
>
>EvalInfo Info(FD->getASTContext(), Status,
> EvalInfo::EM_PotentialConstantExpressionUnevaluated);
> + Info.InConstantContext = true;
>
>// Fabricate a call stack frame to give the arguments a plausible cover
> story.
>ArrayRef Args;
>
> Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp?rev=355743=355742=355743=diff
> ==
> --- cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp (original)
> +++ cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp Fri Mar 8 14:06:48
> 2019
> @@ -1135,3 +1135,27 @@ constexpr bool indirect_builtin_constant
>return __builtin_constant_p(*__s);
> }
> constexpr bool n = indirect_builtin_constant_p("a");
> +
> +__attribute__((enable_if(indirect_builtin_constant_p("a") == n, "OK")))
> +int test_in_enable_if() { return 0; }
> +int n2 = test_in_enable_if();
> +
> +template
> +int test_in_template_param() { return 0; }
> +int n3 = test_in_template_param();
> +
> +void test_in_case(int n) {
> + switch (n) {
> +case indirect_builtin_constant_p("abc"):
> +break;
> + }
> +}
> +enum InEnum1 {
> + ONE = indirect_builtin_constant_p("abc")
> +};
> +enum InEnum2 : int {
> + TWO = indirect_builtin_constant_p("abc")
> +};
> +enum class InEnum3 {
> + THREE = indirect_builtin_constant_p("abc")
> +};
>
> Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=355743=355742=355743=diff
> ==
> --- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
> +++ cfe/trunk/test/SemaCXX/enable_if.cpp Fri Mar 8 14:06:48 2019
> @@ -514,3 +514,11 @@ namespace TypeOfFn {
>
>static_assert(is_same<__typeof__(foo)*, decltype()>::value, "");
> }
> +
> +namespace InConstantContext {
> +void foo(const char *s)
> __attribute__((enable_if(((void)__builtin_constant_p(*s), true), "trap"))) {}
> +
> +void test() {
> + InConstantContext::foo("abc");
> +}
> +} // namespace InConstantContext
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
r355743 - [8.0 Regression] Fix handling of `__builtin_constant_p` inside template arguments, enumerators, case statements, and the enable_if attribute.
Author: ericwf
Date: Fri Mar 8 14:06:48 2019
New Revision: 355743
URL: http://llvm.org/viewvc/llvm-project?rev=355743=rev
Log:
[8.0 Regression] Fix handling of `__builtin_constant_p` inside template
arguments, enumerators, case statements, and the enable_if attribute.
Summary:
The following code is accepted by Clang 7 and prior but rejected by the
upcoming 8 release and in trunk [1]
```
// error {{never produces a constant expression}}
void foo(const char* s) __attribute__((enable_if(__builtin_constant_p(*s) ==
false, "trap"))) {}
void test() { foo("abc"); }
```
Prior to Clang 8, the call to `__builtin_constant_p` was a constant expression
returning false. Currently, it's not a valid constant expression.
The bug is caused because we failed to set `InConstantContext` when attempting
to evaluate unevaluated constant expressions.
[1] https://godbolt.org/z/ksAjmq
Reviewers: rsmith, hans, sbenza
Reviewed By: rsmith
Subscribers: kristina, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59038
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp
cfe/trunk/test/SemaCXX/enable_if.cpp
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=355743=355742=355743=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Mar 8 14:06:48 2019
@@ -11131,6 +11131,7 @@ bool Expr::EvaluateAsConstantExpr(EvalRe
const ASTContext ) const {
EvalInfo::EvaluationMode EM = EvalInfo::EM_ConstantExpression;
EvalInfo Info(Ctx, Result, EM);
+ Info.InConstantContext = true;
if (!::Evaluate(Result.Val, Info, this))
return false;
@@ -11771,6 +11772,7 @@ bool Expr::EvaluateWithSubstitution(APVa
const Expr *This) const {
Expr::EvalStatus Status;
EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantExpressionUnevaluated);
+ Info.InConstantContext = true;
LValue ThisVal;
const LValue *ThisPtr = nullptr;
@@ -11854,6 +11856,7 @@ bool Expr::isPotentialConstantExprUneval
EvalInfo Info(FD->getASTContext(), Status,
EvalInfo::EM_PotentialConstantExpressionUnevaluated);
+ Info.InConstantContext = true;
// Fabricate a call stack frame to give the arguments a plausible cover
story.
ArrayRef Args;
Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp?rev=355743=355742=355743=diff
==
--- cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx1y.cpp Fri Mar 8 14:06:48
2019
@@ -1135,3 +1135,27 @@ constexpr bool indirect_builtin_constant
return __builtin_constant_p(*__s);
}
constexpr bool n = indirect_builtin_constant_p("a");
+
+__attribute__((enable_if(indirect_builtin_constant_p("a") == n, "OK")))
+int test_in_enable_if() { return 0; }
+int n2 = test_in_enable_if();
+
+template
+int test_in_template_param() { return 0; }
+int n3 = test_in_template_param();
+
+void test_in_case(int n) {
+ switch (n) {
+case indirect_builtin_constant_p("abc"):
+break;
+ }
+}
+enum InEnum1 {
+ ONE = indirect_builtin_constant_p("abc")
+};
+enum InEnum2 : int {
+ TWO = indirect_builtin_constant_p("abc")
+};
+enum class InEnum3 {
+ THREE = indirect_builtin_constant_p("abc")
+};
Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=355743=355742=355743=diff
==
--- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
+++ cfe/trunk/test/SemaCXX/enable_if.cpp Fri Mar 8 14:06:48 2019
@@ -514,3 +514,11 @@ namespace TypeOfFn {
static_assert(is_same<__typeof__(foo)*, decltype()>::value, "");
}
+
+namespace InConstantContext {
+void foo(const char *s)
__attribute__((enable_if(((void)__builtin_constant_p(*s), true), "trap"))) {}
+
+void test() {
+ InConstantContext::foo("abc");
+}
+} // namespace InConstantContext
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
