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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
