Commit Graph

21 Commits

Author SHA1 Message Date
Reid Kleckner 345708b681 Revert [Sema] Resolve placeholder types before type deduction to silence spurious `-Warc-repeated-use-of-weak` warnings
This reverts r365382 (git commit 8b1becf2e3)

Appears to regress this semi-reduced fragment of valid code from windows
SDK headers:

  #define InterlockedIncrement64 _InterlockedIncrement64
  extern "C" __int64 InterlockedIncrement64(__int64 volatile *Addend);
  #pragma intrinsic(_InterlockedIncrement64)
  unsigned __int64 InterlockedIncrement(unsigned __int64 volatile *Addend) {
    return (unsigned __int64)(InterlockedIncrement64)((volatile __int64 *)Addend);
  }

Found on a buildbot here, but no mail was sent due to it already being
red:
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/48067

llvm-svn: 365393
2019-07-08 21:59:07 +00:00
Akira Hatanaka 8b1becf2e3 [Sema] Resolve placeholder types before type deduction to silence
spurious `-Warc-repeated-use-of-weak` warnings

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

Differential Revision: https://reviews.llvm.org/D62645

llvm-svn: 365382
2019-07-08 20:04:39 +00:00
Richard Smith 42a22370f2 Revert r350917 "[Sema] If CheckPlaceholderExpr rewrites the initializer
of an auto"

This commit changed the initializer expression passed into
initialization (stripping off an enclosing pair of parentheses or
braces) and subtly changing the meaning of programs, typically by
inserting bogus calls to copy constructors.

See the added testcase in test/SemaCXX/cxx1y-init-captures.cpp for an
example of the breakage.

llvm-svn: 359066
2019-04-24 02:22:38 +00:00
Akira Hatanaka d458ceda24 [Sema] If CheckPlaceholderExpr rewrites the initializer of an auto
variable during auto type deduction, use the rewritten initializer when
performing initialization of the variable.

This silences spurious -Warc-repeated-use-of-weak warnings that are
issued when the initializer uses a weak ObjC pointer.

Differential Revision: https://reviews.llvm.org/D55662

llvm-svn: 350917
2019-01-11 04:57:34 +00:00
Akira Hatanaka 0a84856431 [Sema] Call CheckPlaceholderExpr to resolve typeof or decltype
placeholder expressions while an unevaluated context is still on the
expression evaluation context stack.

This prevents recordUseOfWeek from being called when a weak variable is
used as an operand of a decltype or a typeof expression and fixes
spurious -Warc-repeated-use-of-weak warnings.

rdar://problem/45742525

Differential Revision: https://reviews.llvm.org/D55662

llvm-svn: 350887
2019-01-10 20:12:16 +00:00
Richard Trieu e69acc5d8f Check pointer null-ness before dereferencing it.
-Warc-repeated-use-of-weak may trigger a segmentation fault when the Decl
being checked is outside of a function scope, leaving the current function
info pointer null.  This adds a check before using the function info.

llvm-svn: 333471
2018-05-29 22:43:00 +00:00
Brian Kelley cafd9121cb [Objective-C] Fix "repeated use of weak" warning with -fobjc-weak
Summary: -Warc-repeated-use-of-weak should produce the same warnings with -fobjc-weak as it does with -objc-arc. Also check for ObjCWeak along with ObjCAutoRefCount when recording the use of an evaluated weak variable. Add a -fobjc-weak run to the existing arc-repeated-weak test case and adapt it slightly to work in both modes.

Reviewers: rsmith, doug.gregor, jordan_rose, rjmccall

Reviewed By: rjmccall

Subscribers: arphaman, rjmccall, cfe-commits

Differential Revision: https://reviews.llvm.org/D31005

llvm-svn: 299011
2017-03-29 17:55:11 +00:00
Akira Hatanaka 034c6337e5 [Sema][ObjC] Don't pass a DeclRefExpr that doesn't reference a VarDecl
to WeakObjectProfileTy's constructor.

This fixes an assertion failure in WeakObjectProfileTy's constructor.

rdar://problem/30112633

llvm-svn: 293808
2017-02-01 20:22:26 +00:00
Akira Hatanaka 7e2c82da90 [Objective-c] Do not set IsExact to true when the receiver is a class.
IsExact shouldn't be set to true in WeakObjectProfileTy::getBaseInfo
when the receiver is a class because having a class as the receiver
doesn't guarantee that the Base is exact.

This is a follow-up to r263818.

rdar://problem/25208167

llvm-svn: 264025
2016-03-22 05:00:21 +00:00
Akira Hatanaka 4c62c7c981 [Objective-c] Fix a crash in WeakObjectProfileTy::getBaseInfo.
The crash occurs in WeakObjectProfileTy::getBaseInfo when getBase() is
called on an ObjCPropertyRefExpr object whose receiver is an interface.
This commit fixes the crash by checking the type of the receiver and
setting IsExact to true if it is an interface.

rdar://problem/25208167

Differential Revision: http://reviews.llvm.org/D18268

llvm-svn: 263818
2016-03-18 19:03:50 +00:00
Fariborz Jahanian ac1c5120c6 Objective-C ARC. Fixes a crash when checking for 'weak' propery
whose base is not an expression. rdar://19053620

llvm-svn: 222570
2014-11-21 21:12:11 +00:00
Fariborz Jahanian 6f829e34b3 Objective-C arc: don't count use of __weak
variables when they are used in such unevaluated 
contexts as __typeof, etc. // rdar://13942025

llvm-svn: 182423
2013-05-21 21:20:26 +00:00
Argyrios Kyrtzidis c2091d5d71 Use the extra info in global method pool to speed up looking for ObjC overridden methods.
When we are in a implementation, we check the global method pool whether there were category
methods with the same selector. If there were none (common case) we don't need to do lookups for
overridden methods again.

Note that for an interface method (if we don't encounter its implementation), it is considered that
it overrides methods that were declared before it, not for category methods introduced after it.

This is tradeoff in favor of performance, since it is expensive to do lookups in case there was a
category, and moving the global method pool to ASTContext (so we can check it) would increase complexity.

rdar://13508196

llvm-svn: 179654
2013-04-17 00:09:08 +00:00
Argyrios Kyrtzidis fe7a59d9c2 Revert "Speed-up ObjCMethodDecl::getOverriddenMethods()."
This reverts commit r179436.

Due to caching, it was possible that we could miss overridden methods that
were introduced by categories later on.

Along with reverting the commit I also included a test case that would have caught this.

llvm-svn: 179547
2013-04-15 18:47:22 +00:00
Jordan Rose 25c0ea8995 -Warc-repeated-use-of-weak: allow single reads in loops from local variables.
Previously, the warning would erroneously fire on this:

for (Test *a in someArray)
  use(a.weakProp);

...because it looks like the same property is being accessed over and over.
However, clearly this is not the case. We now ignore loops like this for
local variables, but continue to warn if the base object is a parameter,
global variable, or instance variable, on the assumption that these are
not repeatedly usually assigned to within loops.

Additionally, do-while loops where the condition is 'false' are not really
loops at all; usually they're just used for semicolon-swallowing macros or
using "break" like "goto".

<rdar://problem/12578785&12578849>

llvm-svn: 166942
2012-10-29 17:46:47 +00:00
Jordan Rose b1e3e5f553 -Warc-repeated-use-of-weak: fix a use-of-uninitialized and add a test case.
Fix-up for r165718, should get the buildbots back online.

llvm-svn: 165723
2012-10-11 17:02:00 +00:00
Jordan Rose 76831c6cd4 -Warc-repeated-use-of-weak: Don't warn on a single read followed by writes.
This is a "safe" pattern, or at least one that cannot be helped by using
a strong local variable. However, if the single read is within a loop,
it should /always/ be treated as potentially dangerous.

<rdar://problem/12437490>

llvm-svn: 165719
2012-10-11 16:10:19 +00:00
Jordan Rose 2248765591 -Warc-repeated-use-of-weak: Check messages to property accessors as well.
Previously, [foo weakProp] was not being treated the same as foo.weakProp.
Now, for every explicit message send, we check if it's a property access,
and if so, if the property is weak. Then for every assignment of a
message, we have to do the same thing again.

This is a potentially expensive increase because determining whether a
method is a property accessor requires searching through the methods it
overrides. However, without it -Warc-repeated-use-of-weak will miss cases
from people who prefer not to use dot syntax. If this turns out to be
too expensive, we can try caching the result somewhere, or even lose
precision by not checking superclass methods. The warning is off-by-default,
though.

<rdar://problem/12407765>

llvm-svn: 165718
2012-10-11 16:06:21 +00:00
Jordan Rose e723a27ffe -Warc-repeated-use-of-weak: look through explicit casts on assigned values.
Reading from a weak property, casting the result, and assigning to a
strong pointer should still be considered safe.

llvm-svn: 165629
2012-10-10 16:43:06 +00:00
Jordan Rose 657b5f464d -Warc-repeated-use-of-weak: check ivars and variables as well.
Like properties, loading from a weak ivar twice in the same function can
give you inconsistent results if the object is deallocated between the
two loads. It is safer to assign to a strong local variable and use that.

Second half of <rdar://problem/12280249>.

llvm-svn: 164855
2012-09-28 22:21:35 +00:00
Jordan Rose d393458c33 Add a warning (off by default) for repeated use of the same weak property.
The motivating example:

if (self.weakProp)
  use(self.weakProp);

As with any non-atomic test-then-use, it is possible a weak property to be
non-nil at the 'if', but be deallocated by the time it is used. The correct
way to write this example is as follows:

id tmp = self.weakProp;
if (tmp)
  use(tmp);

The warning is controlled by -Warc-repeated-use-of-receiver, and uses the
property name and base to determine if the same property on the same object
is being accessed multiple times. In cases where the base is more
complicated than just a single Decl (e.g. 'foo.bar.weakProp'), it picks a
Decl for some degree of uniquing and reports the problem under a subflag,
-Warc-maybe-repeated-use-of-receiver. This gives a way to tune the
aggressiveness of the warning for a particular project.

The warning is not on by default because it is not flow-sensitive and thus
may have a higher-than-acceptable rate of false positives, though it is
less noisy than -Wreceiver-is-weak. On the other hand, it will not warn
about some cases that may be legitimate issues that -Wreceiver-is-weak
will catch, and it does not attempt to reason about methods returning weak
values.

Even though this is not a real "analysis-based" check I've put the bug
emission code in AnalysisBasedWarnings for two reasons: (1) to run on
every kind of code body (function, method, block, or lambda), and (2) to
suggest that it may be enhanced by flow-sensitive analysis in the future.

The second (smaller) half of this work is to extend it to weak locals
and weak ivars. This should use most of the same infrastructure.

Part of <rdar://problem/12280249>

llvm-svn: 164854
2012-09-28 22:21:30 +00:00