This is an archive of the discontinued LLVM Phabricator instance.

Fix/Improve SourceRange of explicitly defaulted members
ClosedPublic

Authored by djasper on Jun 17 2014, 4:56 AM.

Details

Reviewers
rsmith
Summary

When adding the implicit compound statement (required for Codegen?), the end location was previously overridden by the start location, probably based on the assumptions:

  • The location of the implicit compound statement should be the member's location
  • The compound statement if present is always the last element of a FunctionDecl

This patch changes the location of the compound statement to the member's end location, but I am open to other suggestions.

Diff Detail

Event Timeline

djasper updated this revision to Diff 10493.Jun 17 2014, 4:56 AM
djasper retitled this revision from to Fix/Improve SourceRange of defaulted destructors.
djasper updated this object.
djasper edited the test plan for this revision. (Show Details)
djasper added a reviewer: rsmith.
djasper added a subscriber: Unknown Object (MLST).
rsmith added inline comments.Jun 18 2014, 1:05 AM
lib/Sema/SemaDeclCXX.cpp
8883

I think this is a reasonable approach. Do the other DefineImplicit* members also need this fix?

djasper updated this revision to Diff 10536.Jun 18 2014, 2:36 AM
djasper retitled this revision from Fix/Improve SourceRange of defaulted destructors to Fix/Improve SourceRange of explicitly defaulted members.
djasper updated this object.
djasper updated this revision to Diff 10553.Jun 18 2014, 5:13 AM

Fall back to getLocation() if getEndLoc() returns an invalid source location (this fixes a few tests for the default copy operators). Also fix a test that seems to have relied on the incorrectly calculated ranges (is the fix right?).

rsmith accepted this revision.Jun 18 2014, 5:35 AM
rsmith edited edge metadata.

When does the !isValid case happen? That sounds like it's probably a bug.

Anyway, LGTM.

This revision is now accepted and ready to land.Jun 18 2014, 5:35 AM

It happens e.g. in test/SemaTemplate/instantiate-default-assignment-operator.cpp. Not sure whether it is a bug or not. I think if the entire member is implicit, getLocation() delivers a somewhat reasonable location whereas getLocEnd() (or EndRangeLoc) is never set.

OK, I suppose that makes some sense (getSourceRange() would then be invalid for implicit things, but getLocation() would be a location to use for diagnostics).

djasper closed this revision.Jun 29 2014, 12:40 PM