This is an archive of the discontinued LLVM Phabricator instance.

[CodeComplete] Add code completion for `delete` and `default` specifier.
ClosedPublic

Authored by lh123 on Jun 25 2020, 7:14 AM.

Details

Summary

add code completion for delete and default specifier.

Diff Detail

Event Timeline

lh123 created this revision.Jun 25 2020, 7:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2020, 7:14 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
lh123 retitled this revision from [CodeComplete] add code completion for `delete` and `default` specifier. to [CodeComplete] Add code completion for `delete` and `default` specifier..Jun 25 2020, 7:27 AM
lh123 updated this revision to Diff 273925.Jun 28 2020, 5:13 AM
lh123 edited the summary of this revision. (Show Details)

Rebase to head.

sammccall added inline comments.Jun 30 2020, 3:33 AM
clang/include/clang/Sema/Sema.h
11933

nit: "specifier" isn't an accurate term here. I don't know that there is one, and it seems better to name the method after the situation rather than the concrete completions it might offer.

Maybe just CodeCompleteAfterFunctionEquals?

clang/lib/Sema/SemaCodeComplete.cpp
6274

I'm not sure offering default whether it's valid or not is an improvement over never offering it.

I think the following checks are easy to implement based on the Declarator and "good enough":

  • offered for destructors
  • offered for operator= (regardless of signature)
  • offered for constructors with 0 or 1 arguments (regardless of type)
  • optional: offered for operator<, operator>, operator==, operator<=> (if C++20 is enabled, even if this is not a member function!)

In particular, we shouldn't offer =default for arbitrary methods (consider = 0 on a virtual method), and we shouldn't offer it for non-members (I think currently it will be offered outside class context)

lh123 updated this revision to Diff 274491.Jun 30 2020, 8:14 AM

Address comments.

lh123 marked 2 inline comments as done.Jun 30 2020, 8:15 AM
lh123 updated this revision to Diff 274496.Jun 30 2020, 8:28 AM

Add the missing operator!=.

sammccall accepted this revision.Jun 30 2020, 8:51 AM

Nice, thank you!

clang/lib/Sema/SemaCodeComplete.cpp
6276

comment: default, copy, and move constructor?

6283

Maybe a comment like "copy and move assignment"

This revision is now accepted and ready to land.Jun 30 2020, 8:51 AM
lh123 marked 2 inline comments as done.Jun 30 2020, 9:47 PM

Hi, your git commit contains extra Phabricator tags. You can drop Reviewers: Subscribers: Tags: and the text Summary: from the git commit with the following script:

arcfilter () {
        arc amend
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people. Please keep the tag. (--date=now is my personal preference (author dates are usually not useful. Using committer dates can make log almost monotonic in time))

https://reviews.llvm.org/D80978 contains a git pre-push hook to automate this.

lh123 added a comment.Jun 30 2020, 9:57 PM

Hi, your git commit contains extra Phabricator tags. You can drop Reviewers: Subscribers: Tags: and the text Summary: from the git commit with the following script:

arcfilter () {
        arc amend
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people. Please keep the tag. (--date=now is my personal preference (author dates are usually not useful. Using committer dates can make log almost monotonic in time))

https://reviews.llvm.org/D80978 contains a git pre-push hook to automate this.

Thanks, should I revert it and recommit it?

This revision was automatically updated to reflect the committed changes.

Hi, your git commit contains extra Phabricator tags. You can drop Reviewers: Subscribers: Tags: and the text Summary: from the git commit with the following script:

arcfilter () {
        arc amend
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people. Please keep the tag. (--date=now is my personal preference (author dates are usually not useful. Using committer dates can make log almost monotonic in time))

https://reviews.llvm.org/D80978 contains a git pre-push hook to automate this.

Thanks, should I revert it and recommit it?

Please don't revert to cause unnecessary churn. Just be mindful when committing for the next time..