[clang] [FlowSensitive] Allow to dump nested RecordStorageLocation (PR #112457)

via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 23 01:03:10 PDT 2024


martinboehme wrote:

I think this is the right change.

What I don't understand, though, is why you were getting an assert failure before. (Which line is the assertion on that failed?) I would have thought if you don't dump the nested record, you just get less information. Apparently not so?

> > I would suggest a brief comment explaining the choice not to filter.
> 
> I'm not sure I understand. There wasn't a choice to filter before, there was just the (incorrect) assumption that we don't have nested `RecordStorageLocation`, leading to a crash.

@ymand I think maybe what you're missing is that the code is calling a different overload of `dump()` now? Previously, it called the `Value` overload, now it's calling the `StorageLocation` overload. And of course the `StorageLocation` doesn't care whether there's a value at the location (or rather, it will figure that out itself and dump the value if so).

@fmayer Just as an explanation of why this check is there: I think this isn't because the code assumed that nested `RecordStorageLocation`s don't exist; rather, we used to have a concept of a `RecordValue`, and when that existed, it made more sense to do this check here. When I eliminated the `RecordValue` concept, I updated some of the code in `ModelDumper` but didn't notice that this check now no longer made sense.

https://github.com/llvm/llvm-project/pull/112457


More information about the cfe-commits mailing list