This could also be used to implement demangling with a fixed-size
temporary storage buffer.
Depends on https://reviews.llvm.org/D50935
Paths
| Differential D51003
Allow demangler's node allocator to fail, and bail out of the entire demangling process when it does. Use this to support a "lookup" query for the mangling canonicalizer that does not create new nodes. ClosedPublic Authored by rsmith on Aug 20 2018, 3:19 PM.
Details Summary This could also be used to implement demangling with a fixed-size Depends on https://reviews.llvm.org/D50935
Diff Detail
Event TimelineComment Actions It seems like we would need to add a lot more nullptr checks in order to really support this then this patch does. One potential workaround is to add a subclass of Node in ItaniumManglingCanonicalizer.cpp that serves as the canonical 'sentinel' node. We could just hand that out whenever we would otherwise allocate anything, and ignore the results of parse() if we ever did. I think that would probably maybe work. Comment Actions
This patch fixes up every place where we create a node and do not immediately return it; all parsing functions returning a Node* are already assumed to potentially fail, so their callers already perform a null check. So I think this is the full extent of what we need to do to support this. (If we wanted to actually make the demangler be able to run with a fixed size buffer, there are two other things we need to address: NodeArray allocation and the explicit malloc/free calls made by PODSmallVector. Those both seem feasible, if we ever want to actually go there.) Comment Actions
Oh, right, my mistake. LGTM after some inline comments.
Ya, I think that would be a good thing to do. It'd also allow us to more gracefully handle failed allocation (right new we just std::terminate()), which would be nice.
This revision is now accepted and ready to land.Aug 23 2018, 1:15 PM Closed by commit rL340670: Allow demangler's node allocator to fail, and bail out of the entire (authored by rsmith). · Explain WhyAug 24 2018, 4:27 PM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 161584 include/llvm/Demangle/ItaniumDemangle.h
include/llvm/Support/ItaniumManglingCanonicalizer.h
lib/Support/ItaniumManglingCanonicalizer.cpp
unittests/Support/ItaniumManglingCanonicalizerTest.cpp
|
Missed a spot here too!