llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.8k stars 11.91k forks source link

default member initializer for 'm' needed within definition of enclosing class for default argument of function #36032

Open JVApen opened 6 years ago

JVApen commented 6 years ago
Bugzilla Link 36684
Version 6.0
OS Windows NT
Attachments Complete Reproduction
CC @Ivan171,@JVApen,@zygoloid,@rnk,@steveire

Extended Description

Following code compiles with MSVC 2015 and not with Clang-Cl:

run.bat

cl.exe /EHsc t.cpp
llvm_6_0_0-RC3\bin\clang-cl.exe -fms-compatibility-version=19 /EHsc t.cpp

t.cpp

#include <limits>
class A
{
   public:
      class B
      {
         public:
            explicit B() = default;
            ~B() = default;

         private:
            double m = std::numeric_limits<double>::max();
      };

   void f(double d, const B &b = B{}) {}
};

int main()
{
   A a{};
   a.f(0.);
}

error

t.cpp(15,34):  error: default member initializer for 'm' needed within definition of enclosing class 'A' outside of member functions
   void f(double d, const B &b = B{}) {}
                                 ^
t.cpp(12,20):  note: default member initializer declared here
            double m = std::numeric_limits<double>::max();
                   ^
1 error generated.
rnk commented 2 years ago

mentioned in issue llvm/llvm-bugzilla-archive#37235

JVApen commented 5 years ago

Correcting 'enhancement' to 'normal' issue. This ain't a feature request.

rnk commented 6 years ago

Bug llvm/llvm-bugzilla-archive#37235 has been marked as a duplicate of this bug.

JVApen commented 6 years ago

Hi Richard, tnx for looking into this. I'm still a bit confused. Even with your explanation and the error message, I don't understand why this code should not compile.

That said, I already worked around this by making this 2 functions (1 calling the other with B as extra argument). So maybe it ain't worth it to be compatible with MSVC for this.

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 6 years ago

Regarding comment#0: if we want to fix this once-and-for-all, we should use the same technique we use for delayed template parsing: teach Sema to call back into the parser to parse the delayed regions on-demand. Then we would only reject the cases where there's an actual dependency cycle.

Regarding comment#2: this is a separate bug. Looks like Clang's support for class-scope explicit specializations is broken (despite our exposing it in MSVC-compatible mode for quite some time). We have to mark such specializations as either "implicit instantiation" or "explicit specialization" (not both, because we only have one TemplateSpecializationKind per specialization); both of those options cause some parts of Clang to do the wrong thing. We'll probably need to add a TSK_ImplicitInstantiationOfExplicitSpecialization and wire that through everywhere to make this properly work.

Ivan171 commented 6 years ago

I got the same error while trying explicit specialization in class scope (DR727 implemented by Richard Smith in r327705).

$ cat > test.cpp template struct outer_t { outer_t() = default;

template <unsigned>
struct inner_t {
    unsigned cache[Size] = {};
    unsigned *store = nullptr;
};

template <>
struct inner_t<0> {
    unsigned *store = nullptr;
};

inner_t<Size> inner;

};

int main() { outer_t<0> outer; }

$ clang++ -std=c++17 test.coo test.cpp:16:19: error: default member initializer for 'store' needed within definition of enclosing class 'outer_t<0>' outside of member functions inner_t inner; ^ test.cpp:13:19: note: default member initializer declared here unsigned *store = nullptr; ^ 1 error generated.

Note: The error only happens if Size is 0. Also, the workaround for explicit specialization in class scope...

template <unsigned, typename = void>
struct inner_t {
    unsigned cache[Size] = {};
    unsigned *store = nullptr;
};

template <typename T>
struct inner_t<0, T> {
    unsigned *store = nullptr;
};

... compiles without any error.

rnk commented 6 years ago

I'm curious what MSVC does if the 'double m' initializer uses a static method from later in 'A'. I think that's legal, and that's why our parsing is done in this order.

Hyperlynx2 commented 2 years ago

This is still broken. Note that it doesn't even need std::numeric_limits to break, you can use any type.

eg:

#include <iostream>

class Foo
{
public:
    struct Bar
    {
        int num = 5;
    };

    Foo(const Bar& bar = Bar{})
    : m_bar(bar)
    {}

    Bar m_bar;
};

int main()
{
    Foo foo;

    std::cout << foo.m_bar.num;
}
rnk commented 2 years ago

GCC agrees with Clang, so this is truly an MSVC compat only issue

luppy commented 2 years ago

GCC agrees with Clang, so this is truly an MSVC compat only issue

Or they just made the same mistake. All it takes to have this issue is two structs and an inline variable. Adding an empty constructor can be used as a workaround.

struct A{
    struct B{
        //B(){} //workaround
        int c = 42;
    };
    static inline B b;
};
llvmbot commented 2 years ago

@llvm/issue-subscribers-clang-frontend

adambadura commented 11 months ago

GCC agrees with Clang, so this is truly an MSVC compat only issue

I'm observing the same problem under Clang 15 whereas GCC 10.2 compiles the same code just fine. A sample (probably too verbose):

#include <utility>

template <typename T, typename... DummyArgs>
class Parameter
{
public:
  template <typename... Args>
  explicit constexpr Parameter(Args&&... args):
    parameter{std::forward<Args>(args)...}
  {
  }

  template <typename... Args>
  static constexpr decltype(auto) make(Args&&... args)
  {
    return Parameter<T, Args...>{std::forward<Args>(args)...};
  }

private:
  T parameter;
};

#define PARAMETER_OBJ(TYPE, NAME, ...) \
  decltype(Parameter<TYPE>::make(__VA_ARGS__)) NAME{__VA_ARGS__}

struct Holder
{
  struct A
  {
    PARAMETER_OBJ(int, value);
  };

  struct B
  {
    PARAMETER_OBJ(A, a);
  };
};

see also https://godbolt.org/z/Pha8x8Td1

It seems a fix that works in my case was to move A and B outside of Holder.

Another workaround is to skip the PARAMETER_OBJ macro and instead use direct syntax like

Parameter<int> value;

and it is enough that at least one of PARAMETER_OBJs gets replaced to make the code compile.

Yet another solution seems to be skipping the make definition. If we keep it like this:

template <typename... Args>
static constexpr Parameter<T, Args...> make(Args&&... args);

then again, the problem goes away.

Endilll commented 11 months ago

Confirmed on post-17 trunk.

llvmbot commented 11 months ago

@llvm/issue-subscribers-c-1

Author: None (JVApen)

| | | | --- | --- | | Bugzilla Link | [36684](https://llvm.org/bz36684) | | Version | 6.0 | | OS | Windows NT | | Attachments | [Complete Reproduction](https://user-images.githubusercontent.com/95090374/143757229-760b5733-b840-4596-9cdd-60169cdc0be8.gz) | | CC | @Ivan171,@JVApen,@zygoloid,@rnk,@steveire | ## Extended Description Following code compiles with MSVC 2015 and not with Clang-Cl: run.bat ======= ``` cl.exe /EHsc t.cpp llvm_6_0_0-RC3\bin\clang-cl.exe -fms-compatibility-version=19 /EHsc t.cpp ``` t.cpp ===== ``` #include <limits> class A { public: class B { public: explicit B() = default; ~B() = default; private: double m = std::numeric_limits<double>::max(); }; void f(double d, const B &b = B{}) {} }; int main() { A a{}; a.f(0.); } ``` error ===== ``` t.cpp(15,34): error: default member initializer for 'm' needed within definition of enclosing class 'A' outside of member functions void f(double d, const B &b = B{}) {} ^ t.cpp(12,20): note: default member initializer declared here double m = std::numeric_limits<double>::max(); ^ 1 error generated. ```
cor3ntin commented 11 months ago

Slightly reduced

class Foo {
public:
    struct Bar {
        int num = 5;
    };

    Foo(Bar bar = Bar{})
    : m_bar(bar) {}

    Bar m_bar;
};

Foo foo;
c0nstexpr commented 10 months ago

Slightly reduced

class Foo {
public:
    struct Bar {
        int num = 5;
    };

    Foo(Bar bar = Bar{})
    : m_bar(bar) {}

    Bar m_bar;
};

Foo foo;

I've just encountered the same problem. Here is a more reduced example:

struct t0 {
    static constexpr struct t1 {
        int i = 0;
    } v0{};
};
shafik commented 9 months ago

This looks like another version of cwg1890 et al which also is the issue in: https://github.com/llvm/llvm-project/issues/68527

I don't think anyone has looked into what it would take to fix this yet.