spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.
There are a few changes I'd recommend to this patch. The biggest one is to
move mangling knowledge from IRExecutionUnit to LanguageCPlusPlus, where it
logically fits better.
The testsuite change should be applied across the board in my opinion, but I'm
going to add Greg Clayton as a reviewer to cover that part.
================
Comment at:
packages/Python/lldbsuite/test/expression_command/call-overloaded-c-fuction/Makefile:9
@@ +8,3 @@
+ifneq (,$(findstring clang,$(CC)))
+ CFLAGS_EXTRAS += -fno-limit-debug-info
+endif
----------------
Should we do this for all tests? **Greg**?
================
Comment at: source/Expression/IRExecutionUnit.cpp:736
@@ +735,3 @@
+ return ConstString(modified_str);
+}
+
----------------
I'm pretty sure this should be on `LanguageCPlusPlus`, along with the code in
`CollectCandidateCPlusPlusNames` that tries `_ZN` -> `_ZNK` and `_Z` -> `_ZL`.
`LanguageCPlusPlus` should have a method that takes a name and collects
"equivalent" mangled names. That way all this mangling knowledge is kept in
one place.
================
Comment at: source/Expression/IRExecutionUnit.cpp:815
@@ -766,1 +814,3 @@
+ ConstString ulong_fixup =
SubstituteMangledParameters(name.AsCString(), llvm::StringRef("y"),
llvm::StringRef("m"));
+ CPP_specs.push_back(SearchSpec(ulong_fixup,
lldb::eFunctionNameTypeFull));
}
----------------
This is going to add `SearchSpec`s regardless of whether
`SubstituteMangledParameters` did anything, slowing down symbol lookups for
cases where it isn't necessary. This should only add specs if there was
actually a difference.
================
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:2081
@@ -2078,1 +2080,3 @@
+ extern_c =
!CPlusPlusLanguage::IsCPPMangledName(function->GetMangled().GetMangledName().AsCString());
+
if (!function_type)
----------------
Two things:
- Why does this have to happen down here? Couldn't it be done up at the
declaration of `extern_c` so that the `bool` stays `const`?
- Looking at this logic, it might also be cool to also set `extern_c` if the
compile unit is C++ but the function in the DWARF has a non-mangled name... but
that's solving a separate problem
Repository:
rL LLVM
http://reviews.llvm.org/D17957
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
