This test was exactly the opposite of what it should be. We should check if
there old decl has linkage (where it makes sense) and if the new decl has
the extern keyword.
Richard Smith [Thu, 4 Apr 2013 01:51:11 +0000 (01:51 +0000)]
Fix 41 of the 61 tests which fail with modules enabled: we were computing and
caching the linkage for a declaration before we set up its redeclaration chain,
when determining whether a declaration could be a redeclaration of something
from an unimported submodule. We actually want to look at the declaration as if
it were not a redeclaration here, so compute the linkage but don't cache it.
John McCall [Thu, 4 Apr 2013 01:38:37 +0000 (01:38 +0000)]
Be sure to check ARC conventions on the implicit method declarations
of a property just in case the property's getter happens to be +1.
We won't synthesize a getter for such a property, but we will allow
the user to define a +1 method for it.
rdar://13115896
Douglas Gregor [Wed, 3 Apr 2013 23:06:26 +0000 (23:06 +0000)]
<rdar://problem/13560075> Teach name lookup for builtin names to find hidden declarations.
Normal name lookup ignores any hidden declarations. When name lookup
for builtin declarations fails, we just synthesize a new
declaration at the point of use. With modules, this could lead to
multiple declarations of the same builtin, if one came from a (hidden)
submodule that was later made visible. Teach name lookup to always
find builtin names, so we don't create these redundant declarations in
the first place.
Richard Smith [Wed, 3 Apr 2013 22:49:41 +0000 (22:49 +0000)]
Pare back r164351 somewhat. The problem that change was addressing was that we
don't serialize a lookup map for the translation unit outside C++ mode, so we
can't tell when lookup within the TU needs to look within modules. Only apply
the fix outside C++ mode, and only to the translation unit.
Anna Zaks [Wed, 3 Apr 2013 21:34:12 +0000 (21:34 +0000)]
[analyzer] Allow tracknullOrUndef look through the ternary operator even when condition is unknown
Improvement of r178684 and r178685.
Jordan has pointed out that I should not rely on the value of the condition to know which expression branch
has been taken. It will not work in cases the branch condition is an unknown value (ex: we do not track the constraints for floats).
The better way of doing this would be to find out if the current node is the right or left successor of the node
that has the ternary operator as a terminator (which is how this is done in other places, like ConditionBRVisitor).
Jordan Rose [Wed, 3 Apr 2013 21:16:58 +0000 (21:16 +0000)]
[analyzer] Correctly handle destructors for lifetime-extended temporaries.
The lifetime of a temporary can be extended when it is immediately bound
to a local reference:
const Value &MyVal = Value("temporary");
In this case, the temporary object's lifetime is extended for the entire
scope of the reference; at the end of the scope it is destroyed.
The analyzer was modeling this improperly in two ways:
- Since we don't model temporary constructors just yet, we create a fake
temporary region when it comes time to "materialize" a temporary into
a real object (lvalue). This wasn't taking base casts into account when
the bindings being materialized was Unknown; now it always respects base
casts except when the temporary region is itself a pointer.
- When actually destroying the region, the analyzer did not actually load
from the reference variable -- it was basically destroying the reference
instead of its referent. Now it does do the load.
This will be more useful whenever we finally start modeling temporaries,
or at least those that get bound to local reference variables.
Anna Zaks [Wed, 3 Apr 2013 19:28:12 +0000 (19:28 +0000)]
[analyzer] Properly handle the ternary operator in trackNullOrUndefValue
1) Look for the node where the condition expression is live when checking if
it is constrained to true or false.
2) Fix a bug in ProgramState::isNull, which was masking the problem. When
the expression is not a symbol (,which is the case when it is Unknown) return
unconstrained value, instead of value constrained to “false”!
(Thankfully other callers of isNull have not been effected by the bug.)
http://lab.llvm.org:8011/builders/clang-x86_64-darwin10-gdb went back green
before it processed the reverted 178663, so it could not have been the culprit.
Objective-C modern rewriter. Fixes a bug
rewriting typedef for a qualified object type
and also when two declarations happen to be on the
same line. // rdar://13562505
[preprocessor] Allow comparing two macro definitions syntactically instead of only lexically.
Syntactically means the function macro parameter names do not need to use the same
identifiers in order for the definitions to be considered identical.
Syntactic equivalence is a microsoft extension for macro redefinitions and we'll also
use this kind of comparison to check for ambiguous macros coming from modules.
Give the default CorrectionCandidateCallback::ValidateCandidate some
smarts so that it doesn't approve of keywords and/or type names when it
knows (based on its flags) that those kinds of corrections are not
wanted.
For variables and functions clang used to store two storage classes. The one
"as written" in the code and a patched one, which, for example, propagates
static to the following decls.
This apparently is from the days clang lacked linkage computation. It is now
redundant and this patch removes it.
Daniel Jasper [Wed, 3 Apr 2013 13:36:17 +0000 (13:36 +0000)]
Improve formatting of for loops and multi-variable DeclStmts.
This combines several related changes:
a) Don't break before after the variable types in for loops with a
single variable.
b) Better indent DeclStmts defining multiple variables.
Even better way to handle comments adjacent to preprocessor directives.
Summary:
It turns out that we don't need to store CommentsBeforeNextToken in the
line state, but rather flush them before we start parsing preprocessor
directives. This fixes wrong comment indentation in code blocks in macro calls
(the test is included).
Richard Trieu [Wed, 3 Apr 2013 02:22:12 +0000 (02:22 +0000)]
Fix a crasher in Template Diffing.
When support was added for declaration arguments, the case of variadic
declaration arguments was not supported. This patch fixes that problem by
not crashing when certain ValueDecl's are null.
Richard Trieu [Wed, 3 Apr 2013 02:11:36 +0000 (02:11 +0000)]
Fix a crasher in Template Diffing.
Value depenedent expressions for default arguments cannot be evaluated.
Instead, use the desugared template type to get an argument expression that
can be used. This is needed for both integer and declaration arguements.
Also, move this common code into a separate function.
From PR9121 gcc defaulted to omitting the frame pointer on linux,
however, it doesn't do that unless we're optimizing. Change
that and haul out to a helper function. Also make this a driver
test appropriate rather than an assembly test.
Jordan Rose [Wed, 3 Apr 2013 01:39:23 +0000 (01:39 +0000)]
Escape more @ signs in Doxygen comments.
Doxygen treats "@command" the same as "\command" in a doc comment, so
whenever we talk about Objective-C things like "@interface" we have to
make sure to escape them.
Jordan Rose [Wed, 3 Apr 2013 01:39:08 +0000 (01:39 +0000)]
[analyzer] Better model for copying of array fields in implicit copy ctors.
- Find the correct region to represent the first array element when
constructing a CXXConstructorCall.
- If the array is trivial, model the copy with a primitive load/store.
- Don't warn about the "uninitialized" subscript in the AST -- we don't use
the helper variable that Sema provides.
John McCall [Wed, 3 Apr 2013 00:56:07 +0000 (00:56 +0000)]
In ObjC++ on legacy runtimes, push an EH cleanup as well as
a normal cleanup when entering a @try or @synchronized to
ensure that we clean that up if an exception is triggered.
Apparently GCC did this, so it's hard to argue that we shouldn't
do at least as much.
Objective-C arc [qui]. Don't issue the bridge cast
warning when doing a __bride cast in non-arc
mode (which has no retain count effect).
// rdar://13514210
Richard Smith [Tue, 2 Apr 2013 19:38:47 +0000 (19:38 +0000)]
If a defaulted special member is implicitly deleted, check whether it's
overriding a non-deleted virtual function. The existing check for this doesn't
catch this case, because it fires before we mark the method as deleted.
Objective-C: Provide fixit hints when warning
about 'isa' ivar being explicitely accessed
when base is a user class object reference.
// rdar://13503456
Fixed "fallthrough annotation does not directly precede switch label" warning in
case when [[clang::fallthrough]]; is used in a method of a local class.
Daniel Jasper [Tue, 2 Apr 2013 14:33:13 +0000 (14:33 +0000)]
Fix some inconsistent use of indentation.
Basically we have always special-cased the top-level statement of an
unwrapped line (the one with ParenLevel == 0) and that lead to several
inconsistencies. All added tests were formatted in a strange way, for
example:
John McCall [Tue, 2 Apr 2013 02:48:58 +0000 (02:48 +0000)]
Add -Wstatic-local-in-inline, which warns about using a static local
variable in a C99 inline (but not static-inline or extern-inline)
function definition.
The standard doesn't actually say that this doesn't apply to
"extern inline" definitions, but that seems like a useful extension,
and it at least doesn't have the obvious flaw that a static
mutable variable in an externally-available definition does.
Anna Zaks [Tue, 2 Apr 2013 01:28:24 +0000 (01:28 +0000)]
[analyzer] Teach invalidateRegions that regions within LazyCompoundVal need to be invalidated
Refactor invalidateRegions to take SVals instead of Regions as input and teach RegionStore
about processing LazyCompoundVal as a top-level “escaping” value.
This addresses several false positives that get triggered by the NewDelete checker, but the
underlying issue is reproducible with other checkers as well (for example, MallocChecker).
Adrian Prantl [Tue, 2 Apr 2013 01:00:48 +0000 (01:00 +0000)]
un-break remaining gdb buildbot testcases.
Make sure we do not generate line info for debugging-related frame setup.
Follow-up to r178361 / rdar://problem/12767564
Jordan Rose [Tue, 2 Apr 2013 00:26:35 +0000 (00:26 +0000)]
[analyzer] For now, don't inline [cd]tors of C++ containers.
This is a heuristic to make up for the fact that the analyzer doesn't
model C++ containers very well. One example is modeling that
'std::distance(I, E) == 0' implies 'I == E'. In the future, it would be
nice to model this explicitly, but for now it just results in a lot of
false positives.
The actual heuristic checks if the base type has a member named 'begin' or
'iterator'. If so, we treat the constructors and destructors of that type
as opaque, rather than inlining them.
This is intended to drastically reduce the number of false positives
reported with experimental destructor support turned on. We can tweak the
heuristic in the future, but we'd rather err on the side of false negatives
for now.
Jordan Rose [Tue, 2 Apr 2013 00:26:29 +0000 (00:26 +0000)]
[analyzer] Cache whether a function is generally inlineable.
Certain properties of a function can determine ahead of time whether or not
the function is inlineable, such as its kind, its signature, or its
location. We can cache this value in the FunctionSummaries map to avoid
rechecking these static properties for every call.
Note that the analyzer may still decide not to inline a specific call to
a function because of the particular dynamic properties of the call along
the current path.
Jordan Rose [Tue, 2 Apr 2013 00:26:26 +0000 (00:26 +0000)]
[analyzer] Use inline storage in the FunctionSummary DenseMap.
The summaries lasted for the lifetime of the map anyway; no reason to
include an extra allocation.
Also, use SmallBitVector instead of BitVector to track the visited basic
blocks -- most functions will have less than 64 basic blocks -- and
use bitfields for the other fields to reduce the size of the structure.
Jordan Rose [Tue, 2 Apr 2013 00:26:15 +0000 (00:26 +0000)]
[analyzer] Allow suppressing diagnostics reported within the 'std' namespace
This is controlled by the 'suppress-c++-stdlib' analyzer-config flag.
It is currently off by default.
This is more suppression than we'd like to do, since obviously there can
be user-caused issues within 'std', but it gives us the option to wield
a large hammer to suppress false positives the user likely can't work
around.
Richard Smith [Mon, 1 Apr 2013 21:43:41 +0000 (21:43 +0000)]
PR15633: Note that we are EnteringContext when parsing the nested name
specifier for an enumeration. Also fix a crash-on-invalid if a non-dependent
name specifier is used to declare an enum template.
Adrian Prantl [Mon, 1 Apr 2013 19:02:06 +0000 (19:02 +0000)]
* Attempt to un-break gdb buildbot by emitting a lexical block end only
when we actually end a lexical block.
* Added new test for line table / block cleanup.
* Follow-up to r177819 / rdar://problem/13115369
John McCall [Mon, 1 Apr 2013 18:34:28 +0000 (18:34 +0000)]
Only merge down a variable type if the previous declaration was
visible. There's a lot of potential badness in how we're modelling
these things, but getting this much correct is reasonably easy.
Thread safety analysis: Turn on checking for non-scalar types by default.
These were previously enabled as a "beta" feature, but they have now been
extensively tested.
Jordan Rose [Sat, 30 Mar 2013 01:31:42 +0000 (01:31 +0000)]
[analyzer] Handle caching out while evaluating a C++ new expression.
Evaluating a C++ new expression now includes generating an intermediate
ExplodedNode, and this node could very well represent a previously-
reachable state in the ExplodedGraph. If so, we can short-circuit the
rest of the evaluation.
projects / clang / log