Page MenuHomePhabricator

[AST] Pick last tentative definition as the acting definition
Needs ReviewPublic

Authored by pestctrl on Apr 1 2021, 7:46 AM.

Details

Reviewers
rsmith
akyrtzi
Summary

Clang currently picks the second tentative definition when
VarDecl::getActingDefinition is called. This can lead to attributes
being dropped if they are attached to tentative definitions that
appear after the second one. This is because
VarDecl::getActingDefinition loops through VarDecl::redecls assuming
that the last tentative definition is the last element in the
iterator. However, it is the second element that would be the last
tentative definition. This changeset modifies getActingDefinition to
iterate through the declaration chain in reverse, so that it can
immediately return when it encounters a tentative definition.

Diff Detail

Event Timeline

pestctrl requested review of this revision.Apr 1 2021, 7:46 AM
pestctrl created this revision.
pestctrl edited the summary of this revision. (Show Details)Apr 1 2021, 7:46 AM
pestctrl edited the summary of this revision. (Show Details)Apr 1 2021, 7:49 AM
pestctrl edited the summary of this revision. (Show Details)Apr 1 2021, 7:52 AM
akyrtzi resigned from this revision.Apr 1 2021, 9:33 AM
rsmith added a comment.Apr 1 2021, 4:12 PM

Thanks, this makes sense.

clang/lib/AST/Decl.cpp
2192

Is there a good reason to switch from checking for a Definition in the loop to checking it ahead of time? This change means we'll do two passes over the redeclarations, and call isThisDeclarationADefinition twice for each, where the old approach only performed one pass.

pestctrl updated this revision to Diff 335113.Apr 3 2021, 3:00 PM
pestctrl edited the summary of this revision. (Show Details)

Removed extra pass over decl chain for acting definition.

clang/lib/AST/Decl.cpp
2192

My thought was that separating the logic would make the loop easier to read, but I see that this causes 2 passes vs. 1.