Page MenuHomePhabricator

[Sema] Resolve placeholder types before type deduction to silence spurious `-Warc-repeated-use-of-weak` warnings
ClosedPublic

Authored by ahatanak on May 29 2019, 9:58 PM.

Details

Summary

The spurious -Warc-repeated-use-of-weak warnings are issued when an initializer expression uses a weak ObjC pointer.

My first attempt to silence the warnings (r350917) caused clang to reject code that is legal in C++17. The patch is based on the feedback I received from Richard when the patch was reverted.

http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190422/268945.html
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190422/268943.html

Diff Detail

Repository
rL LLVM

Event Timeline

ahatanak created this revision.May 29 2019, 9:58 PM
rjmccall added inline comments.May 29 2019, 10:09 PM
lib/Sema/SemaDecl.cpp
11101 ↗(On Diff #202115)

if (Init->hasPlaceholderType(BuiltinType::PseudoObject))

Although maybe this should just be isNonOverloadPlaceholderType(), since it's the syntactic-only placeholders that we're trying to cover. (Feel free to add Expr::hasNonOverloadPlaceholderType() for convenience.)

ahatanak updated this revision to Diff 204889.Jun 14 2019, 6:01 PM
ahatanak marked 2 inline comments as done.

Address review comments.

lib/Sema/SemaDecl.cpp
11101 ↗(On Diff #202115)

I had to move the code in deduceVarTypeFromInitializer that converts UnknownAnyType to id. The conversion has to happen before CheckPlaceholderExpr is called.

rjmccall added inline comments.Thu, Jun 27, 11:05 PM
lib/Sema/SemaDecl.cpp
11101 ↗(On Diff #202115)

Seems reasonable. Can you do the cleanup I requested?

ahatanak updated this revision to Diff 207166.Fri, Jun 28, 4:01 PM
ahatanak marked an inline comment as done.

Add function Expr::hasNonOverloadPlaceholderType.

Thanks, looks much better.

include/clang/Sema/Sema.h
2066 ↗(On Diff #207166)

A shame this needs to be propagated down like this. Is there a reasonable way to just mark on the declaration that we defaulted the type, like an unparseable attribute? This might be interesting to other clients ultimately.

ahatanak updated this revision to Diff 207931.Wed, Jul 3, 5:18 PM
ahatanak marked 2 inline comments as done.

Add an implicit-only attribute ObjCDefaultedAnyToId to avoid passing the flag down.

rjmccall accepted this revision.Wed, Jul 3, 7:29 PM

Thanks, LGTM.

This revision is now accepted and ready to land.Wed, Jul 3, 7:29 PM
rjmccall added inline comments.Wed, Jul 3, 7:30 PM
include/clang/Basic/Attr.td
1844 ↗(On Diff #207931)

Oh, please add a comment on this explaining what it means, since it's not based on a language feature.

This revision was automatically updated to reflect the committed changes.
ahatanak marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptMon, Jul 8, 1:04 PM