JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.
LGTM!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59562/new/
https://reviews.llvm.org/D59562
___
lldb-commits mailing
zturner updated this revision to Diff 191421.
zturner added a comment.
I went ahead and just removed the const `get` version entirely, while changing
the other function name to `getOrLoad` as you suggested. I have another patch
while I'll upload after this one that converts all of the rest of
JDevlieghere added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:30
+
+ const DWARFDataExtractor *maybeLoadArangesData();
+
zturner wrote:
> JDevlieghere wrote:
> > Why do we need these two methods? I presume it's because one
zturner marked an inline comment as done.
zturner added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:30
+
+ const DWARFDataExtractor *maybeLoadArangesData();
+
JDevlieghere wrote:
> Why do we need these two methods? I presume
JDevlieghere added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:30
+
+ const DWARFDataExtractor *maybeLoadArangesData();
+
Why do we need these two methods? I presume it's because one does work and the
other doesn't? If I
zturner created this revision.
zturner added reviewers: JDevlieghere, aprantl, clayborg.
Herald added subscribers: jdoerfert, mgorny.
LLVM's DWARF parsing library has a class called `DWARFContext` which holds all
of the various DWARF data sections and lots of other information. LLDB's on
the
Author: zturner
Date: Tue Mar 19 13:08:56 2019
New Revision: 356509
URL: http://llvm.org/viewvc/llvm-project?rev=356509=rev
Log:
Delete more dead code.
All of this is code that is unreferenced. Removing as much of
this as possible makes it more easy to determine what functionality
is missing
Author: zturner
Date: Tue Mar 19 11:32:43 2019
New Revision: 356495
URL: http://llvm.org/viewvc/llvm-project?rev=356495=rev
Log:
Remove some dead DWARF enum -> string conversion functions.
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDefines.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB356490: Delete dead code. (authored by zturner, committed
by ).
Herald added a project: LLDB.
Changed prior to commit:
https://reviews.llvm.org/D59276?vs=190350=191356#toc
Repository:
rLLDB LLDB
Author: zturner
Date: Tue Mar 19 11:06:32 2019
New Revision: 356490
URL: http://llvm.org/viewvc/llvm-project?rev=356490=rev
Log:
Delete dead code.
Most of these are Dump functions that are never called, but there
is one instance of entire unused classes (DWARFDebugMacinfo and
zturner accepted this revision.
zturner added a comment.
This LGTM, but let's give it a day to see if anyone else chimes in with
comments.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59291/new/
https://reviews.llvm.org/D59291
Author: davide
Date: Tue Mar 19 10:35:40 2019
New Revision: 356487
URL: http://llvm.org/viewvc/llvm-project?rev=356487=rev
Log:
[ScriptInterpreterPython] Remove dead code.
Modified:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
Modified:
Author: davide
Date: Tue Mar 19 10:35:37 2019
New Revision: 356486
URL: http://llvm.org/viewvc/llvm-project?rev=356486=rev
Log:
[StackFrameRecognizer] Remove unneeded LLDB_DISABLE_PYTHON.
Modified:
lldb/trunk/include/lldb/Target/StackFrameRecognizer.h
mgorny added a comment.
I'd also like to update the output of lldb-instr but I think that'd be better
done in a separate patch.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59427/new/
https://reviews.llvm.org/D59427
___
lldb-commits
mgorny added a comment.
Hmm but the tests fail to build for me ;-). I'm going to update this shortly.
Comment at: lldb/source/API/SBAddress.cpp:313
+ LLDB_REGISTER_METHOD(lldb::SBSymbolContext, SBAddress, GetSymbolContext,
+ (uint3_t));
+
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB356469: Remove a couple of log statements. (authored by
zturner, committed by ).
Herald added a subscriber: abidh.
Herald added a project: LLDB.
Changed prior to commit:
Author: zturner
Date: Tue Mar 19 09:26:08 2019
New Revision: 356469
URL: http://llvm.org/viewvc/llvm-project?rev=356469=rev
Log:
Remove a couple of log statements.
These log statements have questionable value, and hinder the effort
of separating the high and low level DWARF parsing interfaces
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.
LGTM. This builds and passes the tests for me. Thanks!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59427/new/
https://reviews.llvm.org/D59427
This revision was automatically updated to reflect the committed changes.
Closed by commit rL356462: Improve error handling for Clang module imports.
(authored by adrian, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
Author: adrian
Date: Tue Mar 19 08:38:26 2019
New Revision: 356462
URL: http://llvm.org/viewvc/llvm-project?rev=356462=rev
Log:
Improve error handling for Clang module imports.
rdar://problem/48883558
Differential Revision: https://reviews.llvm.org/D59524
Added:
Author: labath
Date: Tue Mar 19 08:05:55 2019
New Revision: 356459
URL: http://llvm.org/viewvc/llvm-project?rev=356459=rev
Log:
Fix a "memset clearing an object of non-trivial type" warning in
EmulateInstruction
This is a new warning which started appearing as of gcc-8. The Opcode
class has a
martong added a comment.
> We can't reliable import templates with the ASTImporter, so actually
> reconstructing the template in the debug info AST and then importing it
> doesn't work.
Could you please elaborate how the import of templates fails in ASTImporter? Is
it because the AST you
labath added a comment.
The direction looks good to me too.
Comment at: lldb/source/API/SBAddress.cpp:313
+ LLDB_REGISTER_METHOD(lldb::SBSymbolContext, SBAddress, GetSymbolContext,
+ (uint3_t));
+ LLDB_REGISTER_METHOD(lldb::SBCompileUnit, SBAddress,
balazske added inline comments.
Comment at: lldb/source/Symbol/StdModuleHandler.cpp:220
+case TemplateArgument::Type: {
+ QualType our_type = m_importer.Import(arg.getAsType());
+ imported_args.push_back(TemplateArgument(our_type));
For long-term
lebedev.ri created this revision.
lebedev.ri added a reviewer: courbet.
lebedev.ri added a project: LLVM.
Herald added subscribers: lldb-commits, jdoerfert, abidh, tschuett.
Herald added a project: LLDB.
Let's suppose we have measured 4 different opcodes, and got: `0.5`, `1.0`,
`1.5`, `2.0`.
balazske added a comment.
By the way if the `Import` of the strategy uses recursive import of other
things this can cause same problems as it was in `ASTImporter` before the
`GetImportedOrCreateDecl` was introduced. So this should be avoided or
something similar to `GetImportedOrCreateDecl`
balazske added a comment.
Another observation: The `Import` function of the strategy now has no way to
return an error. An even better version of it would be to include the
possibility of import error (with `ImportError`, or other error type). Or the
"PreImport" function could indicate if the
teemperor planned changes to this revision.
teemperor added a comment.
I think we could also refactor the strategy into a `PreImport` call. That way
we don't break all the user-code out there and it seems less intrusive to the
ASTImporter code. I'll update the patch.
CHANGES SINCE LAST ACTION
teemperor created this revision.
teemperor added reviewers: aprantl, shafik, jingham.
teemperor added a project: C++ modules in LLDB.
Herald added subscribers: lldb-commits, jdoerfert, abidh, mgrang, mgorny.
Herald added a reviewer: martong.
Herald added a reviewer: serge-sans-paille.
Herald added
balazske added a comment.
It looks better now.
One "problem" is that now there is the strategy and there is the `Imported`
function. It would be possible to unify these into a strategy that contains
maybe a "import" and a "post-import" callback. (But this requires big changes
in the
serge-sans-paille added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py:533
+self.assertEqual(2, len(modules))
+self.verify_module(modules[0], "/tmp/a", None)
+
teemperor updated this revision to Diff 191257.
teemperor retitled this revision from "[ASTImporter] Allow adding a shim to the
ASTImporter" to "[ASTImporter] Allow adding a import strategy to the
ASTImporter".
teemperor edited the summary of this revision.
teemperor added a comment.
Thanks for
32 matches
Mail list logo
