[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk added a comment. Thanks for the guidance! Comment at: clang/lib/AST/ExprConstant.cpp:1871-1902 if (!CheckConstantExpression(Info, DiagLoc, EltTy, Value.getArrayInitializedElt(I))) return false; } if (!Value.hasArrayFiller()) return true; return CheckConstantExpression(Info, DiagLoc, EltTy, rsmith wrote: > `Usage` should be passed into these recursive calls to > `CheckConstantExpression`, although that's NFC for now, until/unless we start > allowing class types as template parameters. Makes sense, done. Repository: rC Clang https://reviews.llvm.org/D43320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332018: Allow dllimport non-type template arguments in C++17
(authored by rnk, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43320?vs=146039=146178#toc
Repository:
rC Clang
https://reviews.llvm.org/D43320
Files:
include/clang/AST/Expr.h
lib/AST/ExprConstant.cpp
lib/Sema/SemaOverload.cpp
test/SemaCXX/dllimport-constexpr.cpp
test/SemaCXX/dllimport-memptr.cpp
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -1720,7 +1720,8 @@
/// value for an address or reference constant expression. Return true if we
/// can fold this expression, whether or not it's a constant expression.
static bool CheckLValueConstantExpression(EvalInfo , SourceLocation Loc,
- QualType Type, const LValue ) {
+ QualType Type, const LValue ,
+ Expr::ConstExprUsage Usage) {
bool IsReferenceType = Type->isReferenceType();
APValue::LValueBase Base = LVal.getLValueBase();
@@ -1753,7 +1754,7 @@
return false;
// A dllimport variable never acts like a constant.
- if (Var->hasAttr())
+ if (Usage == Expr::EvaluateForCodeGen && Var->hasAttr())
return false;
}
if (const auto *FD = dyn_cast(VD)) {
@@ -1767,7 +1768,8 @@
// The C language has no notion of ODR; furthermore, it has no notion of
// dynamic initialization. This means that we are permitted to
// perform initialization with the address of the thunk.
- if (Info.getLangOpts().CPlusPlus && FD->hasAttr())
+ if (Info.getLangOpts().CPlusPlus && Usage == Expr::EvaluateForCodeGen &&
+ FD->hasAttr())
return false;
}
}
@@ -1800,12 +1802,14 @@
static bool CheckMemberPointerConstantExpression(EvalInfo ,
SourceLocation Loc,
QualType Type,
- const APValue ) {
+ const APValue ,
+ Expr::ConstExprUsage Usage) {
const ValueDecl *Member = Value.getMemberPointerDecl();
const auto *FD = dyn_cast_or_null(Member);
if (!FD)
return true;
- return FD->isVirtual() || !FD->hasAttr();
+ return Usage == Expr::EvaluateForMangling || FD->isVirtual() ||
+ !FD->hasAttr();
}
/// Check that this core constant expression is of literal type, and if not,
@@ -1843,8 +1847,10 @@
/// Check that this core constant expression value is a valid value for a
/// constant expression. If not, report an appropriate diagnostic. Does not
/// check that the expression is of literal type.
-static bool CheckConstantExpression(EvalInfo , SourceLocation DiagLoc,
-QualType Type, const APValue ) {
+static bool
+CheckConstantExpression(EvalInfo , SourceLocation DiagLoc, QualType Type,
+const APValue ,
+Expr::ConstExprUsage Usage = Expr::EvaluateForCodeGen) {
if (Value.isUninit()) {
Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized)
<< true << Type;
@@ -1863,48 +1869,49 @@
QualType EltTy = Type->castAsArrayTypeUnsafe()->getElementType();
for (unsigned I = 0, N = Value.getArrayInitializedElts(); I != N; ++I) {
if (!CheckConstantExpression(Info, DiagLoc, EltTy,
- Value.getArrayInitializedElt(I)))
+ Value.getArrayInitializedElt(I), Usage))
return false;
}
if (!Value.hasArrayFiller())
return true;
-return CheckConstantExpression(Info, DiagLoc, EltTy,
- Value.getArrayFiller());
+return CheckConstantExpression(Info, DiagLoc, EltTy, Value.getArrayFiller(),
+ Usage);
}
if (Value.isUnion() && Value.getUnionField()) {
return CheckConstantExpression(Info, DiagLoc,
Value.getUnionField()->getType(),
- Value.getUnionValue());
+ Value.getUnionValue(), Usage);
}
if (Value.isStruct()) {
RecordDecl *RD = Type->castAs()->getDecl();
if (const CXXRecordDecl *CD = dyn_cast(RD)) {
unsigned BaseIndex = 0;
- for (CXXRecordDecl::base_class_const_iterator I = CD->bases_begin(),
- End = CD->bases_end(); I != End; ++I, ++BaseIndex) {
-if (!CheckConstantExpression(Info, DiagLoc, I->getType(),
- Value.getStructBase(BaseIndex)))
+ for (const CXXBaseSpecifier : CD->bases()) {
+if (!CheckConstantExpression(Info, DiagLoc, BS.getType(),
+
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/ExprConstant.cpp:1871-1902 if (!CheckConstantExpression(Info, DiagLoc, EltTy, Value.getArrayInitializedElt(I))) return false; } if (!Value.hasArrayFiller()) return true; return CheckConstantExpression(Info, DiagLoc, EltTy, `Usage` should be passed into these recursive calls to `CheckConstantExpression`, although that's NFC for now, until/unless we start allowing class types as template parameters. https://reviews.llvm.org/D43320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk added inline comments.
Comment at: clang/include/clang/AST/Expr.h:662
+ /// Indicates how the constant expression will be used.
+ enum ConstExprUsage { EvaluateForCodeGen, EvaluateForMangling };
+
I expect we could come up with a better name, but is this closer to what you
had in mind?
https://reviews.llvm.org/D43320
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk updated this revision to Diff 146039.
rnk added a comment.
- getting closer
https://reviews.llvm.org/D43320
Files:
clang/include/clang/AST/Expr.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/dllimport-constexpr.cpp
clang/test/SemaCXX/dllimport-memptr.cpp
Index: clang/test/SemaCXX/dllimport-memptr.cpp
===
--- clang/test/SemaCXX/dllimport-memptr.cpp
+++ clang/test/SemaCXX/dllimport-memptr.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++17 %s
// expected-no-diagnostics
Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++14 %s -verify -fms-extensions -triple x86_64-windows-msvc
+// RUN: %clang_cc1 -std=c++17 %s -verify -fms-extensions -triple x86_64-windows-msvc
+
+__declspec(dllimport) void imported_func();
+__declspec(dllimport) int imported_int;
+struct Foo {
+ void __declspec(dllimport) imported_method();
+};
+
+// Instantiation is OK.
+template struct TemplateFnPtr {
+ static void getit() { FP(); }
+};
+template struct TemplateFnRef {
+ static void getit() { FP(); }
+};
+void instantiate1() {
+ TemplateFnPtr<_func>::getit();
+ TemplateFnRef::getit();
+}
+
+// Check variable template instantiation.
+template struct TemplateIntPtr {
+ static int getit() { return *GI; }
+};
+template struct TemplateIntRef {
+ static int getit() { return GI; }
+};
+int instantiate2() {
+ int r = 0;
+ r += TemplateIntPtr<_int>::getit();
+ r += TemplateIntRef::getit();
+ return r;
+}
+
+// Member pointer instantiation.
+template struct TemplateMemPtr { };
+TemplateMemPtr<::imported_method> instantiate_mp;
+
+// constexpr initialization doesn't work for dllimport things.
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (*constexpr_import_func)() = _func;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr int *constexpr_import_int = _int;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (Foo::*constexpr_memptr)() = ::imported_method;
+
+// We make dynamic initializers for 'const' globals, but not constexpr ones.
+void (*const const_import_func)() = _func;
+int *const const_import_int = _int;
+void (Foo::*const const_memptr)() = ::imported_method;
+
+// Check that using a non-type template parameter for constexpr global
+// initialization is correctly diagnosed during template instantiation.
+template struct StaticConstexpr {
+ // expected-error@+1{{must be initialized by a constant expression}}
+ static constexpr void (*g_fp)() = FP;
+};
+void instantiate3() {
+ // expected-note@+1 {{requested here}}
+ StaticConstexpr::g_fp();
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5407,10 +5407,11 @@
SmallVector Notes;
Expr::EvalResult Eval;
Eval.Diag =
+ Expr::ConstExprUsage Usage = CCE == Sema::CCEK_TemplateArg
+ ? Expr::EvaluateForMangling
+ : Expr::EvaluateForCodeGen;
- if ((T->isReferenceType()
- ? !Result.get()->EvaluateAsLValue(Eval, S.Context)
- : !Result.get()->EvaluateAsRValue(Eval, S.Context)) ||
+ if (!Result.get()->EvaluateAsConstantExpr(Eval, Usage, S.Context) ||
(RequireInt && !Eval.Val.isInt())) {
// The expression can't be folded, so we can't keep it at this position in
// the AST.
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -1720,7 +1720,8 @@
/// value for an address or reference constant expression. Return true if we
/// can fold this expression, whether or not it's a constant expression.
static bool CheckLValueConstantExpression(EvalInfo , SourceLocation Loc,
- QualType Type, const LValue ) {
+ QualType Type, const LValue ,
+ Expr::ConstExprUsage Usage) {
bool IsReferenceType = Type->isReferenceType();
APValue::LValueBase Base = LVal.getLValueBase();
@@ -1753,7 +1754,7 @@
return false;
// A dllimport variable never acts like a constant.
- if (Var->hasAttr())
+ if (Usage == Expr::EvaluateForCodeGen && Var->hasAttr())
return false;
}
if (const auto *FD = dyn_cast(VD)) {
@@ -1767,7 +1768,8 @@
// The C language has no notion of ODR; furthermore, it has no notion of
//
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rsmith added inline comments.
Comment at: clang/include/clang/AST/Expr.h:670-672
+ /// Evaluate an expression that is required to be a core constant expression.
+ bool EvaluateAsCoreConstExpr(EvalResult , QualType ParamType,
+ CCEKind CCE, const ASTContext ) const;
rnk wrote:
> rsmith wrote:
> > Seems strange to pass a converted constant expression kind to an 'evaluate
> > as core constant expression' function. And it seems like we don't need the
> > kind here, just an "evaluating for emission w/relocations" vs "evaluating
> > for mangling" enum.
> >
> > Also, we need to evaluate non-type template arguments as constant
> > expressions, not just as core constant expressions, which the
> > implementation does, but the name and comment here don't reflect. (The
> > difference is that you can't result in a pointer/reference to a temporary
> > or automatic / thread storage duration entity.)
> So... what are these things? Converted constant expressions? What are we
> evaluating them as? I guess they're not rvalues or lvalues.
I think this should just be called `EvaluateAsConstantExpr`, and you should
drop all the "core"s throughout. The difference between a constant expression
and a core constant expression is that a core constant expression allows the
evaluated value to refer to entities with automatic storage duration etc, which
is exactly the case you want to disallow here :)
I also don't think you need the `ParamType`, and can instead just look at
whether the expression itself is an rvalue.
Comment at: clang/include/clang/AST/Expr.h:663
+ bool EvaluateAsCoreConstExpr(EvalResult , QualType ParamType,
+ bool IsTemplateArg, const ASTContext )
const;
+
I would prefer an `enum { EvaluateForCodeGen, EvaluateForMangling }` over a
bool `IsTemplateArg`; I would not be surprised if we find other cases where we
want to evaluate an expression for mangling purposes only.
Comment at: clang/lib/AST/ExprConstant.cpp:707
+ /// Evaluate as a C++17 non-type template argument, which is a core
+ /// constant expression with a special case for dllimport declarations.
No "core" here.
Comment at: clang/lib/AST/ExprConstant.cpp:709
+ /// constant expression with a special case for dllimport declarations.
+ EM_TemplateArgument,
+
I don't think we need this. See below.
Comment at: clang/lib/AST/ExprConstant.cpp:10384-10385
+if (!EvaluateLValue(this, LV, Info) || Result.HasSideEffects ||
+!CheckLValueConstantExpression(
+Info, getExprLoc(), Ctx.getLValueReferenceType(getType()), LV))
+ return false;
Instead of a new `EvaluationMode` which is actually not used by evaluation, we
could pass a parameter into `CheckLValueConstantExpression` to indicate if
we're in the "for-mangling" mode.
Comment at: clang/lib/AST/ExprConstant.cpp:10397
+ EvalInfo Info(Ctx, Result, EM);
+ return ::EvaluateAsRValue(Info, this, Result.Val);
+}
If you switch from using `ParamType->isReferenceType()` to asking the
expression for its value category, you don't need the
implicit-lvalue-to-rvalue-conversion semantics of `EvaluateAsRValue`, and can
just call `::Evaluate` here followed by a call to `CheckConstantExpression`
(passing the latter the `IsTemplateArg` flag).
In fact, you don't need the separate case for lvalues above, either, since
`::Evaluate` does the right thing for an lvalue expression by itself.
Comment at: clang/lib/Sema/SemaOverload.cpp:5401
- if ((T->isReferenceType()
- ? !Result.get()->EvaluateAsLValue(Eval, S.Context)
- : !Result.get()->EvaluateAsRValue(Eval, S.Context)) ||
+ if (!Result.get()->EvaluateAsNonTypeTemplateArgument(Eval, T, S.Context) ||
(RequireInt && !Eval.Val.isInt())) {
rnk wrote:
> rsmith wrote:
> > Don't we get here for `CCEKind`s other than the non-type template argument
> > case?
> You're right, but I wasn't able to construct a test case where we would call
> `CheckConvertedConstantExpression` and we would reject it today because it is
> dllimported. This was my best idea, using `if constexpr`:
> ```
> struct __declspec(dllimport) Foo {
> static constexpr bool imported_foo = true;
> };
> const bool some_bool = false;
> const bool *f() {
> if constexpr (Foo::imported_foo) {
> return ::imported_foo;
> } else {
> return _bool;
> }
> }
> ```
>
> Any other ideas on how to get more coverage of this path through
> CheckConvertedConstantExpression?
All the other forms of `CCEKind` also set `RequireInt` to `true`, and so any
case your check catches would also be caught on the line below.
https://reviews.llvm.org/D43320
___
cfe-commits mailing list
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk added inline comments. Comment at: clang/include/clang/AST/Expr.h:670-672 + /// Evaluate an expression that is required to be a core constant expression. + bool EvaluateAsCoreConstExpr(EvalResult , QualType ParamType, + CCEKind CCE, const ASTContext ) const; rsmith wrote: > Seems strange to pass a converted constant expression kind to an 'evaluate as > core constant expression' function. And it seems like we don't need the kind > here, just an "evaluating for emission w/relocations" vs "evaluating for > mangling" enum. > > Also, we need to evaluate non-type template arguments as constant > expressions, not just as core constant expressions, which the implementation > does, but the name and comment here don't reflect. (The difference is that > you can't result in a pointer/reference to a temporary or automatic / thread > storage duration entity.) So... what are these things? Converted constant expressions? What are we evaluating them as? I guess they're not rvalues or lvalues. https://reviews.llvm.org/D43320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk updated this revision to Diff 146025.
rnk added a comment.
- don't expose CCEK, use a bool
https://reviews.llvm.org/D43320
Files:
clang/include/clang/AST/Expr.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/dllimport-constexpr.cpp
clang/test/SemaCXX/dllimport-memptr.cpp
Index: clang/test/SemaCXX/dllimport-memptr.cpp
===
--- clang/test/SemaCXX/dllimport-memptr.cpp
+++ clang/test/SemaCXX/dllimport-memptr.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++17 %s
// expected-no-diagnostics
Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++14 %s -verify -fms-extensions -triple x86_64-windows-msvc
+// RUN: %clang_cc1 -std=c++17 %s -verify -fms-extensions -triple x86_64-windows-msvc
+
+__declspec(dllimport) void imported_func();
+__declspec(dllimport) int imported_int;
+struct Foo {
+ void __declspec(dllimport) imported_method();
+};
+
+// Instantiation is OK.
+template struct TemplateFnPtr {
+ static void getit() { FP(); }
+};
+template struct TemplateFnRef {
+ static void getit() { FP(); }
+};
+void instantiate1() {
+ TemplateFnPtr<_func>::getit();
+ TemplateFnRef::getit();
+}
+
+// Check variable template instantiation.
+template struct TemplateIntPtr {
+ static int getit() { return *GI; }
+};
+template struct TemplateIntRef {
+ static int getit() { return GI; }
+};
+int instantiate2() {
+ int r = 0;
+ r += TemplateIntPtr<_int>::getit();
+ r += TemplateIntRef::getit();
+ return r;
+}
+
+// Member pointer instantiation.
+template struct TemplateMemPtr { };
+TemplateMemPtr<::imported_method> instantiate_mp;
+
+// constexpr initialization doesn't work for dllimport things.
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (*constexpr_import_func)() = _func;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr int *constexpr_import_int = _int;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (Foo::*constexpr_memptr)() = ::imported_method;
+
+// We make dynamic initializers for 'const' globals, but not constexpr ones.
+void (*const const_import_func)() = _func;
+int *const const_import_int = _int;
+void (Foo::*const const_memptr)() = ::imported_method;
+
+// Check that using a non-type template parameter for constexpr global
+// initialization is correctly diagnosed during template instantiation.
+template struct StaticConstexpr {
+ // expected-error@+1{{must be initialized by a constant expression}}
+ static constexpr void (*g_fp)() = FP;
+};
+void instantiate3() {
+ // expected-note@+1 {{requested here}}
+ StaticConstexpr::g_fp();
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5408,9 +5408,8 @@
Expr::EvalResult Eval;
Eval.Diag =
- if ((T->isReferenceType()
- ? !Result.get()->EvaluateAsLValue(Eval, S.Context)
- : !Result.get()->EvaluateAsRValue(Eval, S.Context)) ||
+ if (!Result.get()->EvaluateAsCoreConstExpr(
+ Eval, T, CCE == Sema::CCEK_TemplateArg, S.Context) ||
(RequireInt && !Eval.Val.isInt())) {
// The expression can't be folded, so we can't keep it at this position in
// the AST.
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -704,6 +704,10 @@
/// a constant expression.
EM_PotentialConstantExpression,
+ /// Evaluate as a C++17 non-type template argument, which is a core
+ /// constant expression with a special case for dllimport declarations.
+ EM_TemplateArgument,
+
/// Fold the expression to a constant. Stop if we hit a side-effect that
/// we can't model.
EM_ConstantFold,
@@ -793,6 +797,14 @@
return false;
}
+/// Return true if this declaration is dllimport and we cannot treat the
+/// address as a constant expression. Generally, we do not want to constant
+/// fold dllimport declarations unless they are used in a non-type template
+/// parameter.
+bool isNonConstDllImportDecl(const Decl *D) {
+ return EvalMode != EM_TemplateArgument && D->hasAttr();
+}
+
CallStackFrame *getCallFrame(unsigned CallIndex) {
assert(CallIndex && "no call index in getCallFrame");
// We will eventually hit BottomFrame, which has Index 1, so Frame can't
@@ -845,6 +857,7 @@
LLVM_FALLTHROUGH;
case EM_ConstantExpression:
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rsmith added inline comments.
Comment at: clang/include/clang/AST/Expr.h:103
+/// Contexts in which a converted constant expression is required.
+enum CCEKind {
+ CCEK_CaseValue, ///< Expression in a case label.
If we end up moving this out of `Sema` into the `clang` namespace, it needs a
longer name than this.
Comment at: clang/include/clang/AST/Expr.h:670-672
+ /// Evaluate an expression that is required to be a core constant expression.
+ bool EvaluateAsCoreConstExpr(EvalResult , QualType ParamType,
+ CCEKind CCE, const ASTContext ) const;
Seems strange to pass a converted constant expression kind to an 'evaluate as
core constant expression' function. And it seems like we don't need the kind
here, just an "evaluating for emission w/relocations" vs "evaluating for
mangling" enum.
Also, we need to evaluate non-type template arguments as constant expressions,
not just as core constant expressions, which the implementation does, but the
name and comment here don't reflect. (The difference is that you can't result
in a pointer/reference to a temporary or automatic / thread storage duration
entity.)
https://reviews.llvm.org/D43320
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk updated this revision to Diff 134510.
rnk added a comment.
- revise as discussed
https://reviews.llvm.org/D43320
Files:
clang/include/clang/AST/Expr.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/dllimport-constexpr.cpp
clang/test/SemaCXX/dllimport-memptr.cpp
Index: clang/test/SemaCXX/dllimport-memptr.cpp
===
--- clang/test/SemaCXX/dllimport-memptr.cpp
+++ clang/test/SemaCXX/dllimport-memptr.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++17 %s
// expected-no-diagnostics
Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++14 %s -verify -fms-extensions -triple x86_64-windows-msvc
+// RUN: %clang_cc1 -std=c++17 %s -verify -fms-extensions -triple x86_64-windows-msvc
+
+__declspec(dllimport) void imported_func();
+__declspec(dllimport) int imported_int;
+struct Foo {
+ void __declspec(dllimport) imported_method();
+};
+
+// Instantiation is OK.
+template struct TemplateFnPtr {
+ static void getit() { FP(); }
+};
+template struct TemplateFnRef {
+ static void getit() { FP(); }
+};
+void instantiate1() {
+ TemplateFnPtr<_func>::getit();
+ TemplateFnRef::getit();
+}
+
+// Check variable template instantiation.
+template struct TemplateIntPtr {
+ static int getit() { return *GI; }
+};
+template struct TemplateIntRef {
+ static int getit() { return GI; }
+};
+int instantiate2() {
+ int r = 0;
+ r += TemplateIntPtr<_int>::getit();
+ r += TemplateIntRef::getit();
+ return r;
+}
+
+// Member pointer instantiation.
+template struct TemplateMemPtr { };
+TemplateMemPtr<::imported_method> instantiate_mp;
+
+// constexpr initialization doesn't work for dllimport things.
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (*constexpr_import_func)() = _func;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr int *constexpr_import_int = _int;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (Foo::*constexpr_memptr)() = ::imported_method;
+
+// We make dynamic initializers for 'const' globals, but not constexpr ones.
+void (*const const_import_func)() = _func;
+int *const const_import_int = _int;
+void (Foo::*const const_memptr)() = ::imported_method;
+
+// Check that using a non-type template parameter for constexpr global
+// initialization is correctly diagnosed during template instantiation.
+template struct StaticConstexpr {
+ // expected-error@+1{{must be initialized by a constant expression}}
+ static constexpr void (*g_fp)() = FP;
+};
+void instantiate3() {
+ // expected-note@+1 {{requested here}}
+ StaticConstexpr::g_fp();
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5297,7 +5297,7 @@
/// the converted expression, per C++11 [expr.const]p3.
static ExprResult CheckConvertedConstantExpression(Sema , Expr *From,
QualType T, APValue ,
- Sema::CCEKind CCE,
+ CCEKind CCE,
bool RequireInt) {
assert(S.getLangOpts().CPlusPlus11 &&
"converted constant expression outside C++11");
@@ -5315,7 +5315,7 @@
// condition shall be a contextually converted constant expression of type
// bool.
ImplicitConversionSequence ICS =
- CCE == Sema::CCEK_ConstexprIf
+ CCE == CCEK_ConstexprIf
? TryContextuallyConvertToBool(S, From)
: TryCopyInitialization(S, From, T,
/*SuppressUserConversions=*/false,
@@ -5398,9 +5398,7 @@
Expr::EvalResult Eval;
Eval.Diag =
- if ((T->isReferenceType()
- ? !Result.get()->EvaluateAsLValue(Eval, S.Context)
- : !Result.get()->EvaluateAsRValue(Eval, S.Context)) ||
+ if (!Result.get()->EvaluateAsCoreConstExpr(Eval, T, CCE, S.Context) ||
(RequireInt && !Eval.Val.isInt())) {
// The expression can't be folded, so we can't keep it at this position in
// the AST.
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -649,6 +649,10 @@
/// a constant expression.
EM_PotentialConstantExpression,
+ /// Evaluate as a C++17 non-type template argument, which is a core
+ /// constant expression with a special case for dllimport
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk added inline comments.
Comment at: clang/lib/AST/ExprConstant.cpp:10166-10182
+EvalInfo Info(Ctx, Result, EvalInfo::EM_ConstantFold);
+Info.IsNonTypeTemplateArgument = true;
+LValue LV;
+if (!EvaluateLValue(this, LV, Info) || Result.HasSideEffects ||
+!CheckLValueConstantExpression(
+Info, getExprLoc(), Ctx.getLValueReferenceType(getType()), LV))
+ return false;
rsmith wrote:
> rsmith wrote:
> > Neither `EM_ConstantFold` nor `EM_IgnoreSideEffects` seem like the right
> > evaluation modes to be using to evaluate a non-type template argument. I
> > would expect `EM_ConstantExpression` to be used for both cases.
> Oh, I see, we're allowing side-effects here and then issuing a
> constant-folding warning elsewhere if there actually were any. *sigh*
>
> I would expect we can get away with not doing that for template arguments.
> It'd be worth testing whether GCC actually allows constant folding there, or
> requires a real constant expression.
"Allowing side effects" here looks like it really means allowing non-constant
subexpressions when those subexpressions would be discarded:
```
template struct Foo {};
int x;
Foo<(x, )> f;
constexpr bool is_true = true;
int *fn() {
if constexpr (((void)x, ) != nullptr) {
return
} else {
return nullptr;
}
if constexpr (is_true || x == 2) {
return
} else {
return nullptr;
}
}
```
Clang accepts this, GCC rejects a few. I'm guessing we want that kind of
behavior for `if constexpr` conditions, but maybe not for non-type template
parameters.
So, it sounds like we should check the CCE kind in SemaOverload, and use a more
restrictive constant expression evaluation mode in that case. Perhaps even a
new one for non-type template arguments.
Comment at: clang/lib/Sema/SemaOverload.cpp:5401
- if ((T->isReferenceType()
- ? !Result.get()->EvaluateAsLValue(Eval, S.Context)
- : !Result.get()->EvaluateAsRValue(Eval, S.Context)) ||
+ if (!Result.get()->EvaluateAsNonTypeTemplateArgument(Eval, T, S.Context) ||
(RequireInt && !Eval.Val.isInt())) {
rsmith wrote:
> Don't we get here for `CCEKind`s other than the non-type template argument
> case?
You're right, but I wasn't able to construct a test case where we would call
`CheckConvertedConstantExpression` and we would reject it today because it is
dllimported. This was my best idea, using `if constexpr`:
```
struct __declspec(dllimport) Foo {
static constexpr bool imported_foo = true;
};
const bool some_bool = false;
const bool *f() {
if constexpr (Foo::imported_foo) {
return ::imported_foo;
} else {
return _bool;
}
}
```
Any other ideas on how to get more coverage of this path through
CheckConvertedConstantExpression?
https://reviews.llvm.org/D43320
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rsmith added inline comments.
Comment at: clang/lib/AST/ExprConstant.cpp:10166-10182
+EvalInfo Info(Ctx, Result, EvalInfo::EM_ConstantFold);
+Info.IsNonTypeTemplateArgument = true;
+LValue LV;
+if (!EvaluateLValue(this, LV, Info) || Result.HasSideEffects ||
+!CheckLValueConstantExpression(
+Info, getExprLoc(), Ctx.getLValueReferenceType(getType()), LV))
+ return false;
rsmith wrote:
> Neither `EM_ConstantFold` nor `EM_IgnoreSideEffects` seem like the right
> evaluation modes to be using to evaluate a non-type template argument. I
> would expect `EM_ConstantExpression` to be used for both cases.
Oh, I see, we're allowing side-effects here and then issuing a constant-folding
warning elsewhere if there actually were any. *sigh*
I would expect we can get away with not doing that for template arguments. It'd
be worth testing whether GCC actually allows constant folding there, or
requires a real constant expression.
Comment at: clang/lib/Sema/SemaOverload.cpp:5401
- if ((T->isReferenceType()
- ? !Result.get()->EvaluateAsLValue(Eval, S.Context)
- : !Result.get()->EvaluateAsRValue(Eval, S.Context)) ||
+ if (!Result.get()->EvaluateAsNonTypeTemplateArgument(Eval, T, S.Context) ||
(RequireInt && !Eval.Val.isInt())) {
Don't we get here for `CCEKind`s other than the non-type template argument case?
https://reviews.llvm.org/D43320
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:10166-10182 +EvalInfo Info(Ctx, Result, EvalInfo::EM_ConstantFold); +Info.IsNonTypeTemplateArgument = true; +LValue LV; +if (!EvaluateLValue(this, LV, Info) || Result.HasSideEffects || +!CheckLValueConstantExpression( +Info, getExprLoc(), Ctx.getLValueReferenceType(getType()), LV)) + return false; Neither `EM_ConstantFold` nor `EM_IgnoreSideEffects` seem like the right evaluation modes to be using to evaluate a non-type template argument. I would expect `EM_ConstantExpression` to be used for both cases. https://reviews.llvm.org/D43320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43320: Allow dllimport non-type template arguments in C++17
rnk created this revision.
rnk added a reviewer: rsmith.
When the C++ committee changed the wording around non-type template
arguments, they naively thought that every global declarations address
would be a constant expression, but this is not the case. There is no
PE/COFF relocation that allows using the address of a dllimport global
in another global. Therefore, we do not consider them constexpr, because
users expect that constexpr globals do not require dynamic
initialization.
However, there's nothing that prevents us from doing template
instantiation with dllimported non-type template parameters, so we
should allow it. That's what this patch does.
Fixes PR35772.
At first I tried to implement this as another evaluation mode
'EM_NonTypeTemplateArgument', but I would need rvalue and lvalue
varaints in order to preserve existing behavior around side effects.
https://reviews.llvm.org/D43320
Files:
clang/include/clang/AST/Expr.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/dllimport-constexpr.cpp
clang/test/SemaCXX/dllimport-memptr.cpp
Index: clang/test/SemaCXX/dllimport-memptr.cpp
===
--- clang/test/SemaCXX/dllimport-memptr.cpp
+++ clang/test/SemaCXX/dllimport-memptr.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -verify -std=c++17 %s
// expected-no-diagnostics
Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++14 %s -verify -fms-extensions -triple x86_64-windows-msvc
+// RUN: %clang_cc1 -std=c++17 %s -verify -fms-extensions -triple x86_64-windows-msvc
+
+__declspec(dllimport) void imported_func();
+__declspec(dllimport) int imported_int;
+struct Foo {
+ void __declspec(dllimport) imported_method();
+};
+
+// Instantiation is OK.
+template struct TemplateFnPtr {
+ static void getit() { FP(); }
+};
+template struct TemplateFnRef {
+ static void getit() { FP(); }
+};
+void instantiate1() {
+ TemplateFnPtr<_func>::getit();
+ TemplateFnRef::getit();
+}
+
+// Check variable template instantiation.
+template struct TemplateIntPtr {
+ static int getit() { return *GI; }
+};
+template struct TemplateIntRef {
+ static int getit() { return GI; }
+};
+int instantiate2() {
+ int r = 0;
+ r += TemplateIntPtr<_int>::getit();
+ r += TemplateIntRef::getit();
+ return r;
+}
+
+// Member pointer instantiation.
+template struct TemplateMemPtr { };
+TemplateMemPtr<::imported_method> instantiate_mp;
+
+// constexpr initialization doesn't work for dllimport things.
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (*constexpr_import_func)() = _func;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr int *constexpr_import_int = _int;
+// expected-error@+1{{must be initialized by a constant expression}}
+constexpr void (Foo::*constexpr_memptr)() = ::imported_method;
+
+// We make dynamic initializers for 'const' globals, but not constexpr ones.
+void (*const const_import_func)() = _func;
+int *const const_import_int = _int;
+void (Foo::*const const_memptr)() = ::imported_method;
+
+// Check that using a non-type template parameter for constexpr global
+// initialization is correctly diagnosed during template instantiation.
+template struct StaticConstexpr {
+ // expected-error@+1{{must be initialized by a constant expression}}
+ static constexpr void (*g_fp)() = FP;
+};
+void instantiate3() {
+ // expected-note@+1 {{requested here}}
+ StaticConstexpr::g_fp();
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5398,9 +5398,7 @@
Expr::EvalResult Eval;
Eval.Diag =
- if ((T->isReferenceType()
- ? !Result.get()->EvaluateAsLValue(Eval, S.Context)
- : !Result.get()->EvaluateAsRValue(Eval, S.Context)) ||
+ if (!Result.get()->EvaluateAsNonTypeTemplateArgument(Eval, T, S.Context) ||
(RequireInt && !Eval.Val.isInt())) {
// The expression can't be folded, so we can't keep it at this position in
// the AST.
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -582,6 +582,9 @@
/// we will evaluate.
unsigned StepsLeft;
+/// True if we are evaluating for a non-type template argument.
+bool IsNonTypeTemplateArgument = false;
+
/// BottomFrame - The frame in which evaluation started. This must be
/// initialized after CurrentCall and CallStackDepth.
CallStackFrame BottomFrame;
@@ -738,6 +741,14 @@
return
