Page MenuHomePhabricator

[CMake] Fix ExtensionDependencies.inc with multiple extensions
ClosedPublic

Authored by aeubanks on Nov 17 2020, 9:30 AM.

Details

Summary

When polly is enabled and LLVM_BYE_LINK_INTO_TOOLS=ON is on, ExtensionDependencies.inc does not compile.

$ ninja tools/llvm-config/CMakeFiles/llvm-config.dir/llvm-config.cpp.o
tools/llvm-config/ExtensionDependencies.inc:8:1: error: excess elements in struct initializer
{{"Bye", {"Bye",nullptr}}},

ExtensionDependencies.inc pre-patch:

std::array<ExtensionDescriptor, 2> AvailableExtensions{
{{"Polly", {"support", "core", ...,nullptr}}},
{{"Bye", {"Bye",nullptr}}},
};

ExtensionDependencies.inc with this patch:

std::array<ExtensionDescriptor, 2> AvailableExtensions{
ExtensionDescriptor{"Polly", {"support", "core", ...,nullptr}},
ExtensionDescriptor{"Bye", {"Bye",nullptr}},
};

Diff Detail

Event Timeline

aeubanks created this revision.Nov 17 2020, 9:30 AM
Herald added a project: Restricted Project. · View Herald Transcript
aeubanks requested review of this revision.Nov 17 2020, 9:30 AM
Meinersbur accepted this revision.Nov 17 2020, 2:43 PM

From the array element type, the compiler should know which type to brace-initialize. There seems to be one set of braces too much, makes me wonder how it ever worked.

In any case. the solution looks save, LGTM.

This revision is now accepted and ready to land.Nov 17 2020, 2:43 PM
aeubanks updated this revision to Diff 305935.Nov 17 2020, 5:26 PM

fix whitespace

This revision was landed with ongoing or failed builds.Nov 17 2020, 5:26 PM
This revision was automatically updated to reflect the committed changes.

There seems to be one set of braces too much, makes me wonder how it ever worked.

The « extra » set of brace was for list initialization, using that rule (from https://en.cppreference.com/w/cpp/language/list_initialization)

Otherwise, the constructors of T are considered, in two phases: 

    All constructors that take std::initializer_list as the only argument, or as the first argument if the remaining arguments have default values, are examined, and matched by overload resolution against a single argument of type std::initializer_list 

   If the previous stage does not produce a match, all constructors of T participate in overload resolution against the set of arguments that consists of the elements of the braced-init-list, with the restriction that only non-narrowing conversions are allowed. If this stage produces an explicit constructor  as the best match for a copy-list-initialization, compilation fails (note, in simple copy-initialization, explicit constructors are not considered at all).

Anyway, explicit is better than implicit, so this diff is definitively fine :-)

There seems to be one set of braces too much, makes me wonder how it ever worked.

The « extra » set of brace was for list initialization, using that rule (from https://en.cppreference.com/w/cpp/language/list_initialization)

Otherwise, the constructors of T are considered, in two phases: 

    All constructors that take std::initializer_list as the only argument, or as the first argument if the remaining arguments have default values, are examined, and matched by overload resolution against a single argument of type std::initializer_list 

   If the previous stage does not produce a match, all constructors of T participate in overload resolution against the set of arguments that consists of the elements of the braced-init-list, with the restriction that only non-narrowing conversions are allowed. If this stage produces an explicit constructor  as the best match for a copy-list-initialization, compilation fails (note, in simple copy-initialization, explicit constructors are not considered at all).

Thanks for clarifying.