After implementation of "[Clang] Implement P2741R3 - user-generated static_assert messages" (47ccfd7a89e2a9a747a7114db18db1376324799c) the c indexer crashes when handling a `static_assert` w/o any message.
This is caused by using `dyn_cast` to get the literal string, which isn't working on `nullptr`.
Reviewed By: cor3ntin
Differential Revision: https://reviews.llvm.org/D156053
This is a refactoring of:
- SpacesInConditionalStatement
- SpacesInCStyleCastParentheses
- SpaceInEmptyParentheses
- SpacesInParentheses
These are now options under the new Style Option: SpacesInParens. The
existing options are maintained for backward compatibility.
Within SpacesInParens, there are currently options for:
- Never
- Custom
The currently available options for Custom are:
- InConditionalStatements
- InCStyleCasts
- InEmptyParentheses
- Other
Setting InConditionalStatements and Other to true enables the same space
additions as SpacesInParentheses.
This refactoring does not add or remove any existing features, but it makes
it possible to more easily extend and maintain the addition of spaces within
parentheses.
Related to #55428.
Differential Revision: https://reviews.llvm.org/D155239
This adds a new AlignConsecutiveShortCaseStatements option in line with the
existing AlignConsecutive* options , which when
AllowShortCaseLabelsOnASingleLine is enabled will align the tokens after the
case statement's colon. This also adds a AlignCaseColons option to allow
aligning the case label colon itself rather than the token after it.
Fixes#55475.
Differential Revision: https://reviews.llvm.org/D151761
This adds a new AlignConsecutiveShortCaseStatements option in line with the
existing AlignConsecutive* options , which when
AllowShortCaseLabelsOnASingleLine is enabled will align the tokens after the
case statement's colon. This also adds a AlignCaseColons option to allow
aligning the case label colon itself rather than the token after it.
Fixes#55475.
Differential Revision: https://reviews.llvm.org/D151761
Now that the redundancy between these two classes has been eliminated, these
checks aren't needed any more.
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D155813
After this change, `StructValue` is just a wrapper for an `AggregateStorageLocation`. For the wider context, see https://discourse.llvm.org/t/70086.
## How to review
- Start by looking at the comments added / changed in Value.h, StorageLocation.h,
and DataflowEnvironment.h. This will give you a good overview of the semantic
changes.
- Look at the corresponding .cpp files that implement the semantic changes.
- Transfer.cpp, TypeErasedDataflowAnalysis.cpp, and RecordOps.cpp show how the
core of the framework is affected by the semantic changes.
- UncheckedOptionalAccessModel.cpp shows how this complex model is affected by
the changes.
- Many of the changes in the rest of the patch are mechanical in nature.
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D155446
When tests fail in UncheckedOptionalAccessModelTest.cpp, this prints the name of the optional type instead of a blob of hex.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D155788
This makes it easier to determine which line the unexpected happened on; previously, we would only get the line number.
Reviewed By: ymandel
Differential Revision: https://reviews.llvm.org/D155802
This change provides a centralized record of whether the solver is
"exhausted". In general, once the solver is exhausted, further uses do not
produce meaningful conclusions. Clients can use this information, for example,
to choose to discard all results based on the solver, instead of trying to
distinguish those reached before the limit was reached. Providing it at in the
solver avoids the need to propagate it from each callsite to the client of the
solver.
Differential Revision: https://reviews.llvm.org/D155636
If a non-keyword identifier is found in TypeNames, then a *, &, or && that
follows it is annotated as TT_PointerOrReference.
Differential Revision: https://reviews.llvm.org/D155273
The problem was that the LevelIndentTracker remembered
the indentation level of previous deeper levels when
leaving a scope. Afterwards, when it entered again a
deeper level, it blindly reused the previous
indentation level. In case the --lines option was used
such that the previous deeper level was not formatted,
that previous level was whatever happened to be there
in the source code. The formatter simply believed it.
This is fixed by letting the LevelIndentTracker forget
the previous deeper levels when stepping out of them
(=> change in LevelIndentTracker::nextLine()).
Note that this used to be the case until LLVM 14.0.6,
but was changed in https://reviews.llvm.org/D129064
(#56352) to fix a crash. Our commit here essentially
reverts that crash fix. It was incorrect/incomplete.
Fixes#58464.
Fixes#59178.
Fixes#62799.
Differential Revision: https://reviews.llvm.org/D151047
Instead of asserting merely that the flow condition doesn't imply that a variable is true, make the stronger assertion that the flow condition implies that the variable is false.
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D155067
Fields are imported first and reordered for correct layout.
For partially imported record, layout computation is incorrect.
Differential Revision: https://reviews.llvm.org/D154764
CXXMethodDecl::isVirtual() count the number of overridden methods.
This assertion is not true before overridden methods are fully loaded.
The body of this CXXMethodDecl can introduce deps on a derived class
which contains a method overriding this method, causing the assertion failure.
ImportOverriddenMethods() is moved before body loading to fix this issue.
Testcase is contributed by Balázs Kéri (balazske)
Differential Revision: https://reviews.llvm.org/D154701
In the case where the expression was not yet associated with a storage location, we created a new storage location but failed to associate it with the expression.
The newly added test fails without the fix.
Reviewed By: xazax.hun
Differential Revision: https://reviews.llvm.org/D155465
The ObjC-block detection code only supports a single token as the return type. Add support to detect pointers, too (ObjC has lots of object-pointers).
For example, using `BasedOnStyle: WebKit`, the following is stable output:
```
int* p = ^int*(void)
{ //
return nullptr;
}
();
```
After the patch, this is stable:
```
int* p = ^int*(void) { //
return nullptr;
}();
```
Differential Review: https://reviews.llvm.org/D146434
Besides being a useful abstraction, this function will help insulate existing clients of the framework from upcoming changes to the API of `StructValue` and `AggregateStorageLocation`.
Depends On D155202
Reviewed By: ymandel, xazax.hun
Differential Revision: https://reviews.llvm.org/D155204
The operator keyword preceded by a template closer should be annotated as
TT_FunctionDeclarationName.
Fixes#63879.
Differential Revision: https://reviews.llvm.org/D155358
Fixes two places where we relied on map iteration order when processing
values, which leaked nondeterminism into the generated SAT formulas.
Adds a couple of tests that directly assert that the SAT system is
equivalent on each run.
It's desirable that the formulas are deterministic based on the input:
- our SAT solver is naive and perfermance is sensitive to even simple
semantics-preserving transformations like A|B to B|A.
(e.g. it's likely to choose a different variable to split on).
Timeout failures are bad, but *flaky* ones are terrible to debug.
- similarly when debugging, it's important to have a consistent
understanding of what e.g. "V23" means across runs.
---
Both changes in this patch were isolated from a nullability analysis of
real-world code which was extremely slow, spending ages in the SAT
solver at "random" points that varied on each run.
I've included a reduced version of the code as a regression test.
One of the changes shows up directly as flow-condition nondeterminism
with a no-op analysis, the other relied on bits of the nullability
analysis but I found a synthetic example to show the problem.
Differential Revision: https://reviews.llvm.org/D154948
I added a test for this as the ongoing migration to strict handling of value categories (see https://discourse.llvm.org/t/70086) will change the code that handles this case. It turns out we already didn't handle this correctly, so I fixed the existing implementation.
Depends On D154961
Reviewed By: xazax.hun
Differential Revision: https://reviews.llvm.org/D154965
Previously, we were including these fields only in the specific instance that was initialized by the `InitListExpr`, but not in other instances of the same type. This is inconsistent and error-prone.
Depends On D154952
Reviewed By: xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D154961
These simplify the code in their own right, but they are also useful in that they minimize the number of changes that will need to be made when then API of `AggregateStorageLocation` and `StructValue` changes as part of the migration to strict handling of value categories (see https://discourse.llvm.org/t/70086).
Depends On D154949
Reviewed By: xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D154952
These insulate tests against changes to the `getChild()` functions of `AggregateStorageLocation` and `StructValue` that will happen as part of the migration to strict handling of value categories (see https://discourse.llvm.org/t/70086 for details):
- `AggregateStorageLocation::getChild()` will soon return a `StorageLocation *`
instead of a `StorageLocation &`. When this happens, `getFieldValue()` will be
changed to return null if `AggregateStorageLocation::getChild()` returns null;
test code will not need to change as it should already be checking whether the
return value of `getFieldValue()` is null.
- `StructValue::getChild()` will soon return a `StorageLocation *` instead of a
`Value *`. When this happens, `getFieldValue()` will be changed to look up the
`Value *` in the `Environment`. Again, test code will not need to change.
The test helpers will continue to serve a useful purpose once the API changes are complete, so the intent is to leave them in place.
This patch changes DataflowEnvironmentTest.cpp and RecordOpsTest.cpp to use the test helpers. TransferTest.cpp will be changed in an upcoming patch to help keep patch sizes manageable for review.
Depends On D154934
Reviewed By: ymandel, xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D154935
Soon, it will no longer be possible to default-construct `StructValue`. For details, see https://discourse.llvm.org/t/70086.
For completeness, also add a test that `areEquivalentValues()` on different `IntegerValue`s returns false.
Reviewed By: xazax.hun, gribozavr2
Differential Revision: https://reviews.llvm.org/D154934
This reverts commit 7a72ce98224be76d9328e65eee472381f7c8e7fe.
Test problems were due to unspecified order of function arg evaluation.
Reland "[dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)"
This properly frees the Value hierarchy from managing boolean formulas.
We still distinguish AtomicBoolValue; this type is used in client code.
However we expect to convert such uses to BoolValue (where the
distinction is not needed) or Atom (where atomic identity is intended),
and then fold AtomicBoolValue into FormulaBoolValue.
We also distinguish TopBoolValue; this has distinct rules for
widen/join/equivalence, and top-ness is not represented in Formula.
It'd be nice to find a cleaner representation (e.g. the absence of a
formula), but no immediate plans.
For now, BoolValues with the same Formula are deduplicated. This doesn't
seem desirable, as Values are mutable by their creators (properties).
We can probably drop this for FormulaBoolValue immediately (not in this
patch, to isolate changes). For AtomicBoolValue we first need to update
clients to stop using value pointers for atom identity.
The data structures around flow conditions are updated:
- flow condition tokens are Atom, rather than AtomicBoolValue*
- conditions are Formula, rather than BoolValue
Most APIs were changed directly, some with many clients had a
new version added and the existing one deprecated.
The factories for BoolValues in Environment keep their existing
signatures for now (e.g. makeOr(BoolValue, BoolValue) => BoolValue)
and are not deprecated. These have very many clients and finding the
most ergonomic API & migration path still needs some thought.
Differential Revision: https://reviews.llvm.org/D153469
C89 and C99 list initializers are treated differently than Cpp11 braced
initializers. This patch identifies the C array/struct initializer lists by
finding the preceding equal sign before a left brace, and applies formatting
rules for BracketAlignmentStyle.BlockIndent to those list initializers.
Fixes#57878.
Differential Revision: https://reviews.llvm.org/D153205
This test is failing to compile when LLVM_ENABLE_MODULES=ON due to
NamedDecl being multiply defined. Fix this by avoiding declaring our
own NamedDecl in the test and instead cast a struct of appropriate
size and alignment to NamedDecl.
Fixes a bug that prevents alignment from proceeding through a function
pointer in a list of declarations.
Fixes#63451.
Differential Revision: https://reviews.llvm.org/D153585
Fixes a bug with the handling of right aligned references with left/middle
alignment pointers.
Fixes#63452.
Differential Revision: https://reviews.llvm.org/D153579
My previous fix used a gcc-style attribute, but not all compilers will accept
that. Instead use [[maybe_unused]], which is what we use elsewhere for this
kind of thing.
A stage 2 buildbot that compiles with -Wall -Werror is showing a failure because
a dummy value is unused. Use the unused attribute to suppress the warning, and
add a comment about why we have this value.
UnresolvedSet::erase works by popping the last element then replacing
the element to be erased with that element. When the element to be
erased is itself the last element this leads to writing past the end
of the set, causing an assertion failure.
Fix this by making erase of the last element just pop that element.
Differential Revision: https://reviews.llvm.org/D154502
These changes are OK, but they break downstream stuff that needs more time to adapt :-(
This reverts commit 71579569f4399d3cf6bc618dcd449b5388d749cc.
This reverts commit 5e4ad816bf100b0325ed45ab1cfea18deb3ab3d1.
This reverts commit 1c3ac8dfa16c42a631968aadd0396cfe7f7778e0.
