This member variable was removed a while ago in
443427357f. It was previously used in
materialization code paths that have since been removed. Nowadays,
`m_object_pointer_type` gets set but not used anywhere.
This patch simply removes all remaining instances of it and any
supporting code.
**Testing**
* API tests pass
Differential Revision: https://reviews.llvm.org/D129367
llvm::formatv expects the parameter indexes to start with 0.
Unfortunately it doesn't detect out-of-bounds accesses in the format
string at compile-time, of which we had several inside ClangExpressionDeclMap.
This patch fixes these out-of-bounds format accesses.
Example output
Before
ClangExpressionDeclMap::FindExternalVisibleDecls for '$__lldb_class' in a
'TranslationUnit'
CEDM::FEVD Searching the root namespace
CEDM::FEVD Adding type for $__lldb_class: 1
After
ClangExpressionDeclMap::FindExternalVisibleDecls for '$__lldb_class' in
a 'TranslationUnit'
CEDM::FEVD Searching the root namespace
CEDM::FEVD Adding type for $__lldb_class: class (lambda)
Patch by Michael Buch!
Differential Revision: https://reviews.llvm.org/D128063
Most of our code was including Log.h even though that is not where the
"lldb" log channel is defined (Log.h defines the generic logging
infrastructure). This worked because Log.h included Logging.h, even
though it should.
After the recent refactor, it became impossible the two files include
each other in this direction (the opposite inclusion is needed), so this
patch removes the workaround that was put in place and cleans up all
files to include the right thing. It also renames the file to LLDBLog to
better reflect its purpose.
.. and reduce the scope of others. They don't follow llvm coding
standards (which say they should be used only when the same effect
cannot be achieved with the static keyword), and they set a bad example.
Rather than passing two booleans around, which is especially error prone
with them being next to each other, use a struct with named fields
instead.
Differential revision: https://reviews.llvm.org/D107295
This reverts commit 00764c36ed and the
follow up d2223c7a49.
The original patch broke that one could use static member variables while
inside a static member functions without having a running target. It seems that
LLDB currently requires that static variables are only found via the global
variable lookup so that they can get materialized and mapped to the argument
struct of the expression.
After 00764c36ed static variables of the current
class could be found via Clang's lookup which LLDB isn't observing. This
resulting in expressions actually containing these variables as normal
globals that can't be rewritten to a member of the argument struct.
More specifically, in the test TestCPPThis, the expression
`expr --j false -- s_a` is now only passing if we have a runnable target.
I'll revert the patch as the possible fixes aren't trivial and it degrades
the debugging experience more than the issue that the revert patch addressed.
The underlying bug can be reproduced before/after this patch by stopping
in `TestCPPThis` main function and running: `e -j false -- my_a; A<int>::s_a`.
The `my_a` will pull in the `A<int>` class and the second expression will
be resolved by Clang on its own (which causes LLDB to not materialize the
static variable).
Note: A workaround is to just do `::s_a` which will force LLDB to take the global
variable lookup.
More decoupling of plugins and non-plugins. Target doesn't need to
manage ClangModulesDeclVendor and ClangPersistentVariables is always available
in situations where you need ClangModulesDeclVendor.
Differential Revision: https://reviews.llvm.org/D102811
At the moment the expression parser doesn't support evaluating expressions in
static member functions and just pretends the expression is evaluated within a
non-member function. This causes that all static members are inaccessible when
doing unqualified name lookup.
This patch adds support for evaluating in static member functions. It
essentially just does the same setup as what LLDB is already doing for
non-static member functions (i.e., wrapping the expression in a fake member
function) with the difference that we now mark the wrapping function as static
(to prevent access to non-static members).
Reviewed By: shafik, jarin
Differential Revision: https://reviews.llvm.org/D81550
The comment for ValueType claims that all values <1 are errors, but
not all switch statements take this into account. This patch
introduces an explicit Error case and deletes all default: cases, so
we get warned about incomplete switch coverage.
https://reviews.llvm.org/D96537
It looks like a previous change switched these from LLDB_LOGF but did not update the format strings.
Differential Revision: https://reviews.llvm.org/D96550
Right now we have one large AST for all types in LLDB. All ODR violations in
types we reconstruct are resolved by just letting the ASTImporter handle the
conflicts (either by merging types or somehow trying to introduce a duplicated
declaration in the AST). This works ok for the normal types we build from debug
information as most of them are just simple CXXRecordDecls or empty template
declarations.
However, with a loaded `std` C++ module we have alternative versions of pretty
much all declarations in the `std` namespace that are much more fleshed out than
the debug information declarations. They have all the information that is lost
when converting to DWARF, such as default arguments, template default arguments,
the actual uninstantiated template declarations and so on.
When we merge these C++ module types into the big scratch AST (that might
already contain debug information types) we give the ASTImporter the tricky task
of somehow creating a consistent AST out of all these declarations. Usually this
ends in a messy AST that contains a mostly broken mix of both module and debug
info declarations. The ASTImporter in LLDB is also importing types with the
MinimalImport setting, which usually means the only information we have when
merging two types is often just the name of the declaration and the information
that it contains some child declarations. This makes it pretty much impossible
to even implement a better merging logic (as the names of C++ module
declarations and debug info declarations are identical).
This patch works around this whole merging problem by separating C++ module
types from debug information types. This is done by splitting up the single
scratch AST into two: One default AST for debug information and a dedicated AST
for C++ module types.
The C++ module AST is implemented as a 'specialised AST' that lives within the
default ScratchTypeSystemClang. When we select the scratch AST we can explicitly
request that we want such a isolated sub-AST of the scratch AST. I kept the
infrastructure more general as we probably can use the same mechanism for other
features that introduce conflicting types (such as programs that are compiled
with a custom -wchar-size= option).
There are just two places where we explicitly have request the C++ module AST:
When we export persistent declarations (`$mytype`) and when we create our
persistent result variable (`$0`, `$1`, ...). There are a few formatters that
were previously assuming that there is only one scratch AST which I cleaned up
in a preparation revision here (D92757).
Reviewed By: aprantl
Differential Revision: https://reviews.llvm.org/D92759
Currently one can redefine a persistent variable and LLDB will just silently
ignore the second definition:
```
(lldb) expr int $i = 1
(lldb) expr int $i = 2
(lldb) expr $i
(int) $i = 1
```
This patch makes this an error and rejects the expression with the second
definition.
A nice follow up would be to refactor LLDB's persistent variables to not just be
a pair of type and name, but also contain some way to obtain the original
declaration and source code that declared the variable. That way we could
actually make a full diagnostic as we would get from redefining a variable twice
in the same expression.
Reviewed By: labath, shafik, JDevlieghere
Differential Revision: https://reviews.llvm.org/D89310
Summary:
The ClangASTSource has a lock that globally disables all lookups into the
external AST source when we explicitly "guarded" copy a type. It's not used for
anything else, so importing declarations or importing types that are
dependencies of a declaration actually won't activate that lock. The lookups it
is supposed to prevent also don't actually happen in our test suite. The check
in `ClangExpressionDeclMap::FindExternalVisibleDecls` is never executed and the
check in the `ClangASTSource::FindExternalVisibleDeclsByName` is only ever
reached by the `Import-std-module` tests (which explicitly do a lookup into the
expression context on purpose).
This lock was added in 6abfabff61 as a replacement
for a list of types we already looked up which appeared to be an optimisation
strategy. I assume back then this lock had a purpose but these days the
ASTImporter and LLDB seem to be smart enough to avoid whatever lookups this
tried to prevent.
I would say we remove it from LLDB. The main reason is that it blocks D81561
(which explicitly does a specific lookup to resolve placeholder types produced
by `-flimit-debug-info`) but it's semantics are also very confusing. The naming
implies it's a flag to indicate when we import something at the moment which is
practically never true as described above. Also the fact that it makes our
ExternalASTSource alternate between doing lookups into the debug info and
pretending it doesn't know any external decls could really break our lookup in
some weird way if Clang decides to cache a fake empty lookup result that was
generated while the lock was active.
Reviewers: labath, shafik, JDevlieghere, aprantl
Reviewed By: labath, JDevlieghere, aprantl
Subscribers: aprantl, abidh
Differential Revision: https://reviews.llvm.org/D81749
Types that came from a Clang module are nested in DW_TAG_module tags
in DWARF. This patch recreates the Clang module hierarchy in LLDB and
1;95;0csets the owning module information accordingly. My primary motivation
is to facilitate looking up per-module APINotes for individual
declarations, but this likely also has other applications.
This reapplies the previously reverted commit, but without support for
ClassTemplateSpecializations, which I'm going to look into separately.
rdar://problem/59634380
Differential Revision: https://reviews.llvm.org/D75488
Types that came from a Clang module are nested in DW_TAG_module tags
in DWARF. This patch recreates the Clang module hierarchy in LLDB and
sets the owning module information accordingly. My primary motivation
is to facilitate looking up per-module APINotes for individual
declarations, but this likely also has other applications.
rdar://problem/59634380
Differential Revision: https://reviews.llvm.org/D75488
The size of NameSearchContext isn't important as we never store it and rarely
allocate more than a few. This way we also don't have to use the memset to
initialize these fields to zero.
Summary:
We have a lot of code in our lookup code to pass around `current_id` counters which end up in our logs like this:
```
AOCTV::FT [234] Found XYZ
```
This patch removes all of this code because:
* I'm splitting up all humongous functions, so I need to write more and more boilerplate to pass around these ids.
* I never saw any similar counters in the LLDB/LLVM code base.
* They're essentially globals and the last thing we need in LLDB is even more global state.
* They're not really useful when readings logs. It doesn't help that there isn't just 1 or 2 counters, but 12 (!) unique counters. I always thought that if I see two identical counter values in those brackets it's the same lookup request, but it seems that's only true by accident (and you can't know which of the 12 counters is actually printed without reading the code). The only time I know I can trust the counters is when it's obvious from the log that it's the same counter like in the log below, but then why have the counters in the first place?
```
LayoutRecordType[28] on (ASTContext*)0x00007FFA1C840200 'scratch ASTContext' for (RecordDecl*)0x00007FFA0AAE8CF0 [name = '__tree']
LRT[28] returned:
LRT[28] Original = (RecordDecl*)%p
LRT[28] Size = %lld
LRT[28] Alignment = %lld
LRT[28] Fields:
LRT[28] (FieldDecl*)0x00007FFA1A13B1D0, Name = '__begin_node_', Offset = 0 bits
LRT[28] (FieldDecl*)0x00007FFA1C08FD30, Name = '__pair1_', Offset = 64 bits
LRT[28] (FieldDecl*)0x00007FFA1C061210, Name = '__pair3_', Offset = 128 bits
LRT[28] Bases:
```
Reviewers: labath, shafik, JDevlieghere
Reviewed By: labath, shafik, JDevlieghere
Subscribers: abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74951
Summary:
All of our lookup APIs either use `CompilerDeclContext &` or `CompilerDeclContext *` semi-randomly it seems.
This leads to us constantly converting between those two types (and doing nullptr checks when going from
pointer to reference). It also leads to the confusing situation where we have two possible ways to express
that we don't have a CompilerDeclContex: either a nullptr or an invalid CompilerDeclContext (aka a default
constructed CompilerDeclContext).
This moves all APIs to use references and gets rid of all the nullptr checks and conversions.
Reviewers: labath, mib, shafik
Reviewed By: labath, shafik
Subscribers: shafik, arphaman, abidh, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74607
Summary:
lldb-forward.h is convenient in many ways, but having clang-based
class forward declarations in there makes it easy to proliferate uses of clang
outside of plugins. Removing them makes you much more conscious of when
you're using something from clang and marks where we're using things
from clang in non-plugins.
Differential Revision: https://reviews.llvm.org/D73935
Summary:
This change represents the move of ClangASTImporter, ClangASTMetadata,
ClangExternalASTSourceCallbacks, ClangUtil, CxxModuleHandler, and
TypeSystemClang from lldbSource to lldbPluginExpressionParserClang.h
This explicitly removes knowledge of clang internals from lldbSymbol,
moving towards a more generic core implementation of lldb.
Reviewers: JDevlieghere, davide, aprantl, teemperor, clayborg, labath, jingham, shafik
Subscribers: emaste, mgorny, arphaman, jfb, usaxena95, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73661
A ClangASTSource always has a ClangASTImporter. Let's remove these sporadic
checks with a single assert during construction. They were added originally
for the modern-type-lookup mode that didn't use a ClangASTImporter in there.
Summary:
A *.cpp file header in LLDB (and in LLDB) should like this:
```
//===-- TestUtilities.cpp -------------------------------------------------===//
```
However in LLDB most of our source files have arbitrary changes to this format and
these changes are spreading through LLDB as folks usually just use the existing
source files as templates for their new files (most notably the unnecessary
editor language indicator `-*- C++ -*-` is spreading and in every review
someone is pointing out that this is wrong, resulting in people pointing out that this
is done in the same way in other files).
This patch removes most of these inconsistencies including the editor language indicators,
all the different missing/additional '-' characters, files that center the file name, missing
trailing `===//` (mostly caused by clang-format breaking the line).
Reviewers: aprantl, espindola, jfb, shafik, JDevlieghere
Reviewed By: JDevlieghere
Subscribers: dexonsmith, wuzish, emaste, sdardis, nemanjai, kbarton, MaskRay, atanasyan, arphaman, jfb, abidh, jsji, JDevlieghere, usaxena95, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D73258
Summary:
This commit renames ClangASTContext to TypeSystemClang to better reflect what this class is actually supposed to do
(implement the TypeSystem interface for Clang). It also gets rid of the very confusing situation that we have both a
`clang::ASTContext` and a `ClangASTContext` in clang (which sometimes causes Clang people to think I'm fiddling
with Clang's ASTContext when I'm actually just doing LLDB work).
I also have plans to potentially have multiple clang::ASTContext instances associated with one ClangASTContext so
the ASTContext naming will then become even more confusing to people.
Reviewers: #lldb, aprantl, shafik, clayborg, labath, JDevlieghere, davide, espindola, jdoerfert, xiaobai
Reviewed By: clayborg, labath, xiaobai
Subscribers: wuzish, emaste, nemanjai, mgorny, kbarton, MaskRay, arphaman, jfb, usaxena95, jingham, xiaobai, abidh, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D72684
Summary:
This change is connected with
https://reviews.llvm.org/D69843
In large codebases, we sometimes see Module::FindFunctions (when called from
ClangExpressionDeclMap::FindExternalVisibleDecls) returning huge amounts of
functions.
In current fix I trying to return only function_fullnames from ManualDWARFIndex::GetFunctions when eFunctionNameTypeFull is passed as argument.
Reviewers: labath, jarin, aprantl
Reviewed By: labath
Subscribers: shafik, clayborg, teemperor, arphaman, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D70846
GetPersistentExpressionStateForLanguage() can return a nullptr if it
cannot construct a typesystem. This patch adds missing nullptr checks
at all uses.
Inspired by rdar://problem/58317195
Differential Revision: https://reviews.llvm.org/D72413
GetASTContext is really expensive to call as it makes use of the global
mapping from ASTContext to ClangASTContext. This replaces all calls where
we already have the ClangASTContext around and don't need to call
GetASTContext again.
This adds a check that the ClangASTContext actually fits to the
DeclContext that we want to create a CompilerDeclContext for. If
the ClangASTContext (and its associated ASTContext) does not fit
to the DeclContext (that is, the DeclContext wasn't created by the
ASTContext), all computations using this malformed CompilerDeclContext
will yield unpredictable results.
Also fixes the only place that actually hits this assert which is the
construction of a CompilerDeclContext in ClangExpressionDeclMap
where we pass an unrelated ASTContext instead of the ASTContext
of the current expression.
I had to revert my previous change to DWARFASTParserClangTests.cpp
back to using the unsafe direct construction of CompilerDeclContext
as this assert won't work if the DeclContext we pass isn't a valid
DeclContext in the first place.
The CompilerDeclContext constructor takes a void* pointer which
means that all callers of this constructor need to first explicitly
convert all pointers to clang::DeclContext*. This causes that we
for example can't just pass a TranslationUnitDecl* to the constructor without
first casting it to its parent class (as it inherits from both
Decl and DeclContext so the void* pointer is actually a Decl*).
This patch introduces a utility function in the ClangASTContext
which gets rid of the requirement to cast all pointers to
clang::DeclContext. Also moves all constructor calls to use this
function instead which is NFC (beside the change in
DWARFASTParserClangTests.cpp).
ClangASTContext::getASTContext() currently returns a ptr but we have an assert there since a
while that the ASTContext is not a nullptr. This causes that we still have a lot of code
that is doing nullptr checks on the result of getASTContext() which is all unreachable code.
This patch changes the return value to a reference to make it clear this can't be a nullptr
and deletes all the nullptr checks.
This adds a unit test for looking up persistent declarations in the scratch AST
context. Also adds the `GetPersistentDecl` hook to the ClangExpressionDeclMap
that this unit test can emulate looking up persistent variables without having
a lldb_private::Target.
The ClangExpressionDeclMap should be testable from a unit test. This is currently
impossible as they have both dependencies on Target/ExecutionContext from their
constructor. This patch allows constructing these classes without an active Target
and adds the missing tests for running without a target that we can do at least
a basic lookup test without crashing.
Summary:
As discussed on the mailing list [1] we have to make a decision for how to proceed with the modern-type-lookup.
This patch removes modern-type-lookup from LLDB. This just removes all the code behind the modern-type-lookup
setting but it does *not* remove any code from Clang (i.e., the ExternalASTMerger and the clang-import-test stay around
for now).
The motivation for this is that I don't think that the current approach of implementing modern-type-lookup
will work out. Especially creating a completely new lookup system behind some setting that is never turned on by anyone
and then one day make one big switch to the new system seems wrong. It doesn't fit into the way LLVM is developed and has
so far made the transition work much more complicated than it has to be.
A lot of the benefits that were supposed to come with the modern-type-lookup are related to having a better organization
in the way types move across LLDB and having less dependencies on unrelated LLDB code. By just looking at the current code (mostly
the ClangASTImporter) I think we can reach the same goals by just incrementally cleaning up, documenting, refactoring
and actually testing the existing code we have.
[1] http://lists.llvm.org/pipermail/lldb-dev/2019-December/015831.html
Reviewers: shafik, martong
Subscribers: rnkovacs, christof, arphaman, JDevlieghere, usaxena95, lldb-commits, friss
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71562
The overloads that don't take a CompilerType serve no purpose as we
always have a CompilerType in the scope where we call them. Instead
just call the overload that takes a CompilerType and delete the
now unused other overloaded methods.
Target doesn't really need to know about ClangASTContext more than any
other TypeSystem. We can create a method ClangASTContext::GetScratch for
anything who needs a ClangASTContext specifically instead of just a
generic TypeSystem.
Summary: Not once have I looked at these numbers in a log and considered them useful. Also this should not have been implemented via an unguarded list of globals.
Reviewers: martong, shafik
Reviewed By: shafik
Subscribers: rnkovacs, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D71336
This code was just creating a new SymbolContextList with any found functions
in the front and orders them by how close they are to the current frame.
This refactors this code into its own function to make this more obvious.
Doesn't do any other changes to the code, so this is NFC.
Zequan Wu