Page MenuHomePhabricator

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

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



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.

Diff Detail


Event Timeline

ahatanak created this revision.May 29 2019, 9:58 PM
rjmccall added inline comments.May 29 2019, 10:09 PM
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.

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.Jun 27 2019, 11:05 PM
11101 ↗(On Diff #202115)

Seems reasonable. Can you do the cleanup I requested?

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

Add function Expr::hasNonOverloadPlaceholderType.

Thanks, looks much better.

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.Jul 3 2019, 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.Jul 3 2019, 7:29 PM

Thanks, LGTM.

This revision is now accepted and ready to land.Jul 3 2019, 7:29 PM
rjmccall added inline comments.Jul 3 2019, 7:30 PM
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 TranscriptJul 8 2019, 1:04 PM