static constexpr member function of a local struct in a function template is instantiated too late

Clang Frontend
1

Hello,

I’m new to clang development and was hoping to get some help in tackling this bug that I found/filed: http://llvm.org/bugs/show_bug.cgi?id=20625

Example:

template
void F() {
struct Num {
static constexpr int Get() { return 42; }
};
constexpr int n = Num::Get();
}
int main() {
F();
}

The summary of the issue is that the local struct Num and its static constexpr function Get is not yet instantiated when we validate the constexpr variable initialization of n during F’s instantiation.

More details:

Num and Get gets added to PendingLocalImplicitInstantiations in SubstStmt at SemaTemplateInstantiationDecl.cpp:3437, and is instantiated later by PerformPendingInstantiations at SemaTemplateInstantiationDecl.cpp:3458. However, the validation of the constexpr variable initialization of n happens in SubstStmt at which point we don’t yet have the definition of Get instantiated.

I’m not exactly sure what the correct approach would be to solve the problem. It seems that the local implicit instantiations need to be performed earlier, or the validation of constexpr variables need to be delayed.

If someone can point me in the right direction it would be appreciated.

Thanks,

Michael Park

2

ping

3

Hello,

Hi and welcome! (And sorry for the delay.)

I'm new to clang development and was hoping to get some help in tackling
this bug that I found/filed: 20625 – static constexpr member function of a local struct in a function template is instantiated too late

Example:

template <typename T>

void F() {
  struct Num {
    static constexpr int Get() { return 42; }
  };
  constexpr int n = Num::Get();
}
int main() {
  F<int>();
}

The summary of the issue is that the local struct *Num* and its static
constexpr function *Get* is not yet instantiated when we validate the
constexpr variable initialization of *n* during *F*'s instantiation.

More details:

*Num* and *Get* gets added to *PendingLocalImplicitInstantiations* in
*SubstStmt* at *SemaTemplateInstantiationDecl.cpp:3437*, and is
instantiated later by *PerformPendingInstantiations* at
*SemaTemplateInstantiationDecl.cpp:3458*. However, the validation of the
constexpr variable initialization of *n* happens in *SubstStmt* at which
point we don't yet have the definition of *Get* instantiated.

I'm not exactly sure what the correct approach would be to solve the
problem. It seems that the local implicit instantiations need to be
performed earlier, or the validation of constexpr variables need to be
delayed.

If someone can point me in the right direction it would be appreciated.

The implicit instantiations should be performed sooner. (FWIW, the same
problem will affect member functions of local classes that have deduced
return types.) Perhaps the easiest way to address this would be to perform
all the local implicit instantiations discovered while instantiating a
local class when we reach the end of that local class, instead of delaying
them until the end of the surrounding function. We try to delay these
instantiations as much as we can, to reduce the stack usage in recursive
template instantiation, but that's probably not going to be significant
here because there's not likely to be many layers of AST between the
function and the class definition.

One place to put this fix would be around
SemaTemplateInstantiateDecl.cpp:1104:

  // DR1484 clarifies that the members of a local class are instantiated as
part
  // of the instantiation of their enclosing entity.
  if (D->isCompleteDefinition() && D->isLocalClass()) {
    SemaRef.InstantiateClass(D->getLocation(), Record, D, TemplateArgs,
                             TSK_ImplicitInstantiation,
                             /*Complain=*/true);
    SemaRef.InstantiateClassMembers(D->getLocation(), Record, TemplateArgs,
                                    TSK_ImplicitInstantiation);
  }

You could save, perform, and restore the pending local implicit
instantiations around these calls.

4

Hi Richard, thanks for the reply!

Funnily enough, the member functions of local classes that have deduced return types actually don’t run into this problem because the definition gets instantiated eagerly in order to deduce the return type.
So the following actually works in clang+±3.4.2.

template
void F() {
struct Num {
static constexpr auto Get() { return 42; }
};
static_assert(Num::Get() == 42, “”);
}
int main() {
F();
}

I tried the recommended approach of instantiating at the end of the local class instead, like so:

// DR1484 clarifies that the members of a local class are instantiated as part
// of the instantiation of their enclosing entity.
if (D->isCompleteDefinition() && D->isLocalClass()) {
SemaRef.InstantiateClass(D->getLocation(), Record, D, TemplateArgs,
TSK_ImplicitInstantiation,
/Complain=/true);
SemaRef.InstantiateClassMembers(D->getLocation(), Record, TemplateArgs,
TSK_ImplicitInstantiation);

SemaRef.PerformPendingInstantiations(/LocalOnly=/true);

}

Although there are PeformPendingInstantiations calls that should be removed, it effectively will be a no-op so I just tried this out for now.
It actually passes make test and also passes the example above!

But I worry that instantiating at the end of the local class is still too late, because I run into the same issue if I bring the static_assert into the body of the local struct.

template

void F() {

struct Num {
static constexpr int Get() { return 42; }
static_assert(range == 42, “”);
};

}

This case also works if I change the return type of Get to auto.

Even I bring out the local struct to namespace scope, it still breaks. But I think this might be a different bug. (In this case changing Get’s return type to auto doesn’t resolve the problem.)

struct Num {
static constexpr int Get() { return 42; }
static_assert(Get() == 42, “”);
};

P.S. Thanks so much for helping out.

5

Hi Richard, thanks for the reply!

Funnily enough, the member functions of local classes that have deduced
return types actually don't run into this problem because the definition
gets instantiated eagerly in order to deduce the return type.
So the following actually works in clang++-3.4.2.

template <typename T>

void F() {
  struct Num {
    static constexpr auto Get() { return 42; }
  };
  static_assert(Num::Get() == 42, "");

}
int main() {
  F<int>();
}

I tried the recommended approach of instantiating at the end of the local
class instead, like so:

// DR1484 clarifies that the members of a local class are instantiated as

part
// of the instantiation of their enclosing entity.
if (D->isCompleteDefinition() && D->isLocalClass()) {
  SemaRef.InstantiateClass(D->getLocation(), Record, D, TemplateArgs,
                           TSK_ImplicitInstantiation,
                           /*Complain=*/true);
  SemaRef.InstantiateClassMembers(D->getLocation(), Record, TemplateArgs,
                                  TSK_ImplicitInstantiation);

* SemaRef.PerformPendingInstantiations(/*LocalOnly=*/true);*

}

Although there are *PeformPendingInstantiations *calls that should be
removed, it effectively will be a no-op so I just tried this out for now.
It actually passes *make test* and also passes the example above!

But I worry that instantiating at the end of the local class is still too
late, because I run into the same issue if I bring the *static_assert* into
the body of the local struct.

template <typename T>

void F() {

  struct Num {

    static constexpr int Get() { return 42; }
    static_assert(range == 42, "");
  };

}

The core language is somewhat unclear on whether that's supposed to work.
Note that it doesn't work if F is not a template. I think the standard
committee will end up deciding that this case is ill-formed, so I'm happy
for us to reject it for now.

This case also works if I change the return type of *Get* to *auto*.

Even I bring out the local struct to namespace scope, it still breaks. But
I think this might be a different bug. (In this case changing *Get*'s
return type to *auto* doesn't resolve the problem.)

=) Yep. See C++ Standard Core Language Active Issues

6

I’ve followed http://llvm.org/docs/DeveloperPolicy.html#one-off-patches and http://llvm.org/docs/GettingStarted.html#sending-patches-with-git to send an email to cfe-commits mailing list with a git diff as an attachment. Compared to other emails though my seem to look different so I think I may have done it wrong?

7

I've followed LLVM Developer Policy — LLVM 18.0.0git documentation
and Getting Started with the LLVM System — LLVM 18.0.0git documentation to
send an email to *cfe-commits* mailing list with a *git diff* as an
attachment. Compared to other emails though my seem to look different so
I think I may have done it wrong?

It's fine. One minor thing: we prefer -p0 patches, and git produces -p1
patches by default. Use 'git diff --no-prefix'.

8

Ah, ok. Thanks Richard!