-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Clang] Instantiate variables referenced in decltype
with an undeduced type.
#161231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang Author: Corentin Jabot (cor3ntin) ChangesFixes #160497 Full diff: https://github.com/llvm/llvm-project/pull/161231.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 270b5d336eba7..3ff40f98334ca 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -432,6 +432,7 @@ Bug Fixes to C++ Support
- Fix an assertion failure when taking the address on a non-type template parameter argument of
object type. (#GH151531)
- Suppress ``-Wdouble-promotion`` when explicitly asked for with C++ list initialization (#GH33409).
+- Correctly deduced return types in ``decltype`` expressions. (#GH160497)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3b267c1b1693d..3302bfce193a2 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -20108,8 +20108,9 @@ static void DoMarkVarDeclReferenced(
bool NeededForConstantEvaluation =
isPotentiallyConstantEvaluatedContext(SemaRef) && UsableInConstantExpr;
- bool NeedDefinition =
- OdrUse == OdrUseContext::Used || NeededForConstantEvaluation;
+ bool NeedDefinition = OdrUse == OdrUseContext::Used ||
+ NeededForConstantEvaluation ||
+ Var->getType()->isUndeducedType();
assert(!isa<VarTemplatePartialSpecializationDecl>(Var) &&
"Can't instantiate a partial template specialization.");
diff --git a/clang/test/SemaCXX/decltype.cpp b/clang/test/SemaCXX/decltype.cpp
index 739485b57a3ec..971cf5132d4d5 100644
--- a/clang/test/SemaCXX/decltype.cpp
+++ b/clang/test/SemaCXX/decltype.cpp
@@ -170,3 +170,33 @@ class conditional {
// FIXME: The diagnostics here are produced twice.
void foo(conditional<decltype((1),int>) { // expected-note 2 {{to match this '('}} expected-error {{expected ')'}} expected-note 2{{to match this '<'}}
} // expected-error {{expected function body after function declarator}} expected-error 2 {{expected '>'}} expected-error {{expected ')'}}
+
+
+namespace GH160497 {
+
+template <class> struct S {
+ template <class>
+ inline static auto mem =
+ [] { static_assert(false); // expected-error {{static assertion failed}} \
+ // expected-note {{while substituting into a lambda expression here}}
+ return 42;
+ }();
+};
+
+using T = decltype(S<void>::mem<void>);
+ // expected-note@-1 {{in instantiation of static data member 'GH160497::S<void>::mem<void>' requested here}}
+
+namespace N1 {
+
+template<class>
+struct S {
+ template<class>
+ inline static auto mem = 42;
+};
+
+using T = decltype(S<void>::mem<void>);
+
+T y = 42;
+
+}
+}
|
OdrUse == OdrUseContext::Used || NeededForConstantEvaluation; | ||
bool NeedDefinition = OdrUse == OdrUseContext::Used || | ||
NeededForConstantEvaluation || | ||
Var->getType()->isUndeducedType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually wonder if this should be isUndeducedAutoType
. This ends up doing a type visitor to check if there is ANY 'auto' in the type (GetContainedDeducedTypeVisitor
).
Do we expect this to work with an auto*
return type to the lambda, etc? I actually lean towards YES, but would love a test or two?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we condition this check on TSK != TSK_Undeclared
? As it stands, every time any variable is mentioned we'll walk its type looking for auto
, which sounds quite expensive, given how common name references to variables are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, note that we do have the same check inside this function
if (UsableInConstantExpr || Var->getType()->isUndeducedType()) {
So it would be at least worth combining it into a bool variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw that, I'll make a PR
inline static auto mem = | ||
[] { static_assert(false); // expected-error {{static assertion failed}} \ | ||
// expected-note {{while substituting into a lambda expression here}} | ||
return 42; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test ONLY tests auto
as a return, but the change itself is ANY type containing an undeduced type. Perhaps there is value in a -> auto *
, ->auto []
, function pointer return, and `pack expansion' type?
Does this patch fix #161196? |
@zwuis Yes, thanks! |
Address post commit feedback from llvm#161231
Address post commit feedback from #161231
…ced type. (llvm#161231) Fixes llvm#160497 Fixes llvm#56652 Fixes llvm#116319 Fixes llvm#161196
…1648) Address post commit feedback from llvm#161231
Fixes #160497
Fixes #56652
Fixes #116319
Fixes #161196