bloomberg / clang-p2996

Experimental clang support for WG21 P2996 (Reflection).
https://github.com/bloomberg/clang-p2996/tree/p2996/P2996.md
62 stars 15 forks source link

Expanding results in repeated first argument on Windows #120

Open HaydnTrigg opened 6 days ago

HaydnTrigg commented 6 days ago

Describe the bug Trying to expand the members of a struct that contains 'Cheese', 'Bacon', 'Burger'. Compiling for Linux results in expected output, for Windows however 'Cheese' is repeated 3 times.

Expected behavior in Godbolt https://godbolt.org/z/3fo8zGGbc

Observed behavior

Cheese
Cheese
Cheese

Expected behavior

Cheese
Bacon
Burger

To Reproduce I am compiling this test program using clang-cl and overriding the system headers to include libcxx before MSVC.

The source code

#include <experimental/meta>
#include <vector>
#include <iostream>

// start 'expand' definition
namespace __impl
{
    template <auto... vals>
    struct replicator_type
    {
        template <typename F>
        constexpr void operator>>(F body) const
        {
            (body.template operator()<vals>(), ...);
        }
    };

    template <auto... vals>
    replicator_type<vals...> replicator = {};
}

template <typename R>
consteval auto expand(R range)
{
    std::vector<std::meta::info> args;
    for (auto r : range)
    {
        args.push_back(std::meta::reflect_value(r));
    }
    return substitute(^^__impl::replicator, args);
}
// end 'expand' definition

struct MyStruct
{
    std::string Cheese;
    std::string Bacon;
    std::string Burger;
};

int main(int argc, const char *argv[])
{
    [:expand(nonstatic_data_members_of(^^MyStruct)):] >> [&]<auto dm>
    {
        using T = typename[:type_of(dm):];
        std::cout << identifier_of(dm) << std::endl;
    };

    return 0;
}

Relevant command line -std=c++26 -freflection -freflection-new-syntax -freflection-latest

Environment (please complete the following information):

Additional context I have added some extra logging to substitute that spits out some output just before executing SetAndSucceed and the output from the compile option and program is as follows.

StructuralValue: meta::info ReflectionKind:4 Name:Cheese
StructuralValue: meta::info ReflectionKind:4 Name:Bacon
StructuralValue: meta::info ReflectionKind:4 Name:Burger
StructuralValue: meta::info ReflectionKind:4 Name:Cheese
StructuralValue: meta::info ReflectionKind:4 Name:Bacon
StructuralValue: meta::info ReflectionKind:4 Name:Burger
Cheese
Cheese
Cheese
// just before SetAndSucceed add P(TArgs);
auto P = [](SmallVector<TemplateArgument, 4>& TArgs) {
  for(auto TArg : TArgs)
  {
    if (TArg.getKind() == TemplateArgument::StructuralValue) {
        const QualType T = TArg.getStructuralValueType();
        const APValue & RV = TArg.getAsStructuralValue();

        std::string Name;
        if (auto *ND = dyn_cast<NamedDecl>(RV.getReflectedDecl())) {
          if (auto *II = ND->getIdentifier())
            Name = II->getName();
          else if (auto *II = ND->getDeclName().getCXXLiteralIdentifier())
            Name = II->getName();
        }

        llvm::outs() << "StructuralValue: " << T.getAsString() << " ReflectionKind:" << (int)RV.getReflectionKind() << " Name:" << Name << "\n";
    } else {
        llvm::outs() << "Non-type template argument. " << TArg.getKind() << "\n";
    }
  }
};
HaydnTrigg commented 6 days ago

I think I’ve made some progress in diagnosing the issue. I finally bothered to set up a proper development environment and ran a debug build. This revealed errors in previously unreachable sections of the code within the MicrosoftMangler. (Seems sketchy that unreachable code doesn't explode in a release build but continues to produce... something? even if it's invalid output with zero warning that there might have been a problem)

image

I 'implemented' ReflectionKind::Declaration by copy-pasting from the ItaniumMangler and, well, hacking it until it compiled and hey presto this seems to have resolved the issue. I'm guessing that the mangled names were being encoded identically, so it just ended up always using the first symbol, I might double check that later.

Link to my branch with changes https://github.com/HaydnTrigg/clang-p2996/tree/microsoft-mangling

I assume that everything is just going to be placeholder for the moment with name mangling. I imagine it's going to be a very long time before there is a spec for mangling reflection types and I want to be able to test the reflection code on Windows.

Would it be welcomed if I implemented some placeholder logic for reflection in the MicrosoftMangler code for the time being and submitted a pull request for that?