This is an archive of the discontinued LLVM Phabricator instance.

[modules] Do not report missing definitions of demoted constexpr variable templates. This is a followup to regression introduced in r284284. This should fix our libstdc++ modules builds.
ClosedPublic

Authored by v.g.vassilev on Oct 17 2016, 7:43 AM.

Details

Reviewers
bkramer
rsmith

Diff Detail

Event Timeline

v.g.vassilev retitled this revision from to [modules] Do not report missing definitions of demoted constexpr variable templates. This is a followup to regression introduced in r284284. This should fix our libstdc++ modules builds..
v.g.vassilev updated this object.
v.g.vassilev added reviewers: rsmith, bkramer.
v.g.vassilev set the repository for this revision to rL LLVM.
v.g.vassilev added a subscriber: cfe-commits.
manmanren added inline comments.
lib/Sema/SemaDecl.cpp
10129

Is the logic correct here? The if statement says !Var->isThisDeclarationADemotedDefinition(), and we then assert Var->isThisDeclarationADemotedDefinition() && getLangOpts().Modules.

v.g.vassilev removed rL LLVM as the repository for this revision.
v.g.vassilev marked an inline comment as done.
v.g.vassilev added inline comments.
lib/Sema/SemaDecl.cpp
10129

Oops, that is an old version of the patch. Uploading the new one.

rsmith added inline comments.Oct 18 2016, 10:49 AM
lib/Sema/SemaDecl.cpp
10129

Can you just remove this assertion entirely? I don't see why it's relevant to what this code is checking. If we had some other reason to demote, the logic here would still seem to be correct.

v.g.vassilev marked an inline comment as done.

Remove assert.

rsmith accepted this revision.Oct 18 2016, 5:33 PM
rsmith edited edge metadata.
This revision is now accepted and ready to land.Oct 18 2016, 5:33 PM
v.g.vassilev closed this revision.Oct 19 2016, 4:28 AM

Landed in r284577.