- User Since
- Oct 8 2012, 9:19 AM (280 w, 2 d)
Another note/thought: It'd be great if, when including the whole file, there was some way to omit the snippet marker comments - but, on looking around at the Sphinx docs I don't see anything immediately obvious & figure there's nothing specifically to support that (I guess if someone cared a lot they could add new build rules that would strip the comments & produce some temporary files that could then be included when the whole source listing was desired).
As for a name (& might be worth using it here for the MCOption too) - maybe just "DwarfRangesBase" (Not sure if a "Use" prefix is helpful/necessary?), with the intent of the flag being -fdwarf-ranges-base
Tue, Feb 20
What's the motivation for this change? Do you have a platform that doesn't support debug_string?
Given the abundance of callers that know the call won't fail, perhaps it'd be appropriate to keep the original function name without Expected, then have a version of the function ('tryBlah') that returns Expected (& implement blah in terms of cantFail(tryBlah))
Thu, Feb 15
Seems awkward/not ideal to compute the start/end, then reverse those computations to compute the values appropriate to print in verbose mode - probably better to keep the original values & compute the final values when needed/printing?
Wed, Feb 14
Seems good, thanks :)
Mon, Feb 12
Also, if possible - the Entry dtor could be made non-virtual and protected in the base class, if these objects are never owned polymorphically, which it /looks/ like they aren't, but not entirely sure.
Sun, Feb 11
Not sure what you mean by "a non-syscall diagnostic"? (there's StringError if you want to just put a string rather than an error code)
Fri, Feb 9
Curious combination of tests - some assembly, some IR. I think I made the existing test cases all checked in binaries.
Thu, Feb 8
Ideally/the original version of this, I think was non-recursive and only specifically looked for certain attributes in order and then stopped. I'd sort of rather than, because it's more constrained.
The original find() still exists, so I'd expect this variant only to be used where the recursive property is needed.
I'm guessing this is insufficient for catching indirect recursion (DIE A refers to DIE B and B back to A)?
Wed, Feb 7
Adrian - what do you think about Reid's point regarding not changing the C API unless needed? I'm fairly OK with that, but not sure. Seems like it could be a surprising "gotcha" where someone thinks they're doing the right things to produce CodeView but they're missing this because the API isn't exposed/suggesting that usage.
Does this need to be the name of an actual object file? Any idea how this should work in the case of LTO?
Mon, Feb 5
Seems plausible - maybe walking the scopes to count the range-for loops would produce a better number, but would be slower. :/ Dunno.
Fri, Feb 2
Few things that could tidy up the tests a bit, but otherwise looks good :)
Looks good - thanks!
Thu, Feb 1
Looks good to me
Wed, Jan 31
Tue, Jan 30
Mon, Jan 29
Thu, Jan 25
Sounds alright - thanks (:
Wed, Jan 24
I'm guessing several of these invalid tests could be in one file - only the "length too long" should need to be last in the list, right? ("length too short" is undetectable, isn't it? it'd just appear as though the remaining bytes outside the length were the next entry and that entry would be incorrect in some way (its length, version, etc))
Tue, Jan 23
Jan 22 2018
Jan 18 2018
Jan 16 2018
So, to summarize the changes for myself/others, focusing on the changes to the metadata syntax/semantics (rather than the DIBuilder API changes - though they're equivalent):
Jan 15 2018
Test case? (maybe just a static_assert with std::is_same type trait or something in the unit test file)
Looks good to me
Jan 10 2018
Thanks for CCing me Matt - but yeah, this seems reasonable/makes sense to me!
Jan 9 2018
Jan 8 2018
This looks like it's depending on implementation details of LLVM's libSupport (by redeclaring them locally). Best not to do that - can this code be changed to use the public interface of libSupport (the stuff in include/llvm/Support/Regex.h)? Or, if necessary, expand that interface to provide the necessary functionality explicitly in the header.
Jan 5 2018
Jan 4 2018
Fine by me - this'll also be necessary/useful for the upcoming trivial_abi attribute that's being proposed in clang, I think.
Sure, seems plausible
Dec 22 2017
Seems good to me
Dec 20 2017
Dec 19 2017
Dec 18 2017
Good for me - addressing/removing the base class & type traits test before committing.
Dec 15 2017
Dec 14 2017
Yeah, can't say I know much of anything about this. @echristo might, or might be able to point to someone who would be suitable.
I'm pretty OK with this - haven't thought /deeeply/ about the algorithm from the code (more from the discussions we've had), mostly glossed over, looked at the code style, etc, seems plausible.
Dec 7 2017
Code mostly looks plausible. Haven't quite understood the "ParentIntervals*" variables/processing.
Dec 4 2017
Just a quick note:
Nov 29 2017
Not totally clear to me why this feature is desirable, but assume you've got use cases/reasons :)
Nov 27 2017
Nov 25 2017
Happy to hear from Adrian and Paul if this doesn't seem like the right direction - but can be handled post-commit too.