Bugzilla Link	42513
Resolution	FIXED
Resolved on	Nov 01, 2020 13:11
Version	trunk
OS	All
Blocks	#41819
Reporter	LLVM Bugzilla Contributor
CC	@DougGregor,@zmodem,@zygoloid

Extended Description

Commit r350505 (#38883 ) introduced a regression for the following code:

/*******************************************************************/
#include <stdio.h>

struct Widget {
virtual ~Widget() = default;
virtual void doAction() const = 0;

template <typename X, typename... Args>
static Widget* Construct(Args... args) {
constexpr auto i = GetWidgetCtor(0);
if constexpr (i != nullptr)
return i(args...);
else
return nullptr;
}

private:
template <typename X, auto Ret = WidgetCtor((X*)nullptr)>
static constexpr auto GetWidgetCtor(X* = nullptr) {
return Ret;
}

template
static constexpr auto GetWidgetCtor(...) {
return nullptr;
}
};

template <typename... Args>
using WidgetGenerator = Widget*(*)(Args...);

template <typename X, typename... Args>
struct StandardWidget : Widget {
virtual void doAction() const override {
x.doAction();
}

private:
friend constexpr WidgetGenerator<Args...> WidgetCtor(X*) {
return [](Args... args){
return static_cast<Widget*>(new StandardWidget(X(args...)));
};
}

StandardWidget(X x) : x(x) { }
X x;
};

struct X1 {
friend constexpr WidgetGenerator<> WidgetCtor(X1*);

X1() { }

void doAction() const {
puts("X1::doAction");
}
};

struct X2 {
friend constexpr WidgetGenerator WidgetCtor(X2*);

X2(int i) : i(i) { }

void doAction() const {
printf("X2::doAction with %i\n", i);
}

int i;
};

struct X3 {
friend constexpr WidgetGenerator<> WidgetCtor(X3*);

X3(double d) : d(d) { }
double d;
};

template struct StandardWidget;
template struct StandardWidget<X2, int>;

struct X3Widget : Widget {
virtual void doAction() const override {
printf("X3::doAction with %f\n", x.d);
}

private:
friend constexpr WidgetGenerator<> WidgetCtor(X3*) {
return {
return static_cast<Widget*>(new X3Widget());
};
}

X3Widget() : x(12.5) { }
X3 x;
};

void test() {
if (Widget* i = Widget::Construct())
i->doAction(), delete i; // "X1::doAction"
if (Widget* i = Widget::Construct(123))
i->doAction(), delete i; // "X2::doAction with 123"
if (Widget* i = Widget::Construct())
i->doAction(), delete i; // "X3::doAction with 12.5"
if (Widget* i = Widget::Construct())
i->doAction(), delete i; // no-op
}
/*******************************************************************/

Rather than outputting three lines, only one is now output ("X3::doAction with 12.5"). The code compiled correctly on clang 7 and earlier and also compiles correctly on gcc.

The relevant output (-S -emit-llvm -O2) for the test() function in clang 8 and later is:

define dso_local void @​_Z4testv() local_unnamed_addr #​0 personality i32 (...)* @​__gxx_personality_v0 !dbg !​135 {
%1 = tail call i32 (i8*, ...) @​printf(i8* getelementptr inbounds ([22 x i8], [22 x i8]* @.str.2, i64 0, i64 0), double 1.250000e+01), !dbg !​147
ret void, !dbg !​174
}

And for clang 7 and earlier is:

define dso_local void @​_Z4testv() local_unnamed_addr #​0 personality i32 (...)* @​__gxx_personality_v0 !dbg !​136 {
%1 = tail call i32 @​puts(i8* getelementptr inbounds ([13 x i8], [13 x i8]* @.str, i64 0, i64 0)), !dbg !​148
%2 = tail call i32 (i8*, ...) @​printf(i8* getelementptr inbounds ([22 x i8], [22 x i8]* @.str.1, i64 0, i64 0), i32 123), !dbg !​151
%3 = tail call i32 (i8*, ...) @​printf(i8* getelementptr inbounds ([22 x i8], [22 x i8]* @.str.2, i64 0, i64 0), double 1.250000e+01), !dbg !​154
ret void, !dbg !​181
}

In the #38231 , I posted the above code and the following trivial fix:

Index: lib/AST/DeclBase.cpp

--- lib/AST/DeclBase.cpp (revision 350505)
+++ lib/AST/DeclBase.cpp (working copy)
@@ -1407,7 +1407,8 @@

// Skip friends and local extern declarations unless they're the first
// declaration of the entity.

• if ((D->isLocalExternDecl() || D->getFriendObjectKind()) &&

• if ((D->isLocalExternDecl() ||

•  D->getFriendObjectKind() == Decl::FOK_Declared) && D != D->getCanonicalDecl()) 

  return true;

Richard Smith commented on the above that "The proposed fix doesn't seem problematic, but only changes which declaration of the friend will be in the DeclContext name lookup table ... we should walk its redeclarations and find the one with the body" but unfortunately I am not able to work out the best method to do this, so I have posted this bug to enable someone else to take up the burden if they can see a good solution :o)