[PATCH] D113633: [llvm] Add support for DW_TAG_immutable_type

Luís Ferreira via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 12 09:49:31 PST 2021


ljmf00 added a comment.

In D113633#3127708 <https://reviews.llvm.org/D113633#3127708>, @probinson wrote:

> Typically I've done "support" for a tag or other DWARF feature in several phases, each usually (not necessarily) with its own patch.  Each phase builds on the previous one, which is the project's preferred incremental-development model.
>
> - First, do enough so that llvm-dwarfdump can correctly handle an object file with a DW_TAG_immutable_type in it; this generally means tweaking lib/DebugInfo/DWARF a little bit, as I see you have done.  The test can be an assembler file with the raw bytes listed; there are many examples of this.
> - Next, make it so textual IR can describe an immutable type, which can be emitted into an object file. The IR part might involve round-tripping through textual and bitcode formats to show that all works. For the object-file part you use llvm-dwarfdump, because your previous work verified that llvm-dwarfdump handles the tag correctly.
> - Next, if you need to fiddle with DIBuilder to get the IR API to handle immutable types, you do that, probably with a unittest (not a Clang test, because LLVM has other frontend clients and LLVM testing should not depend on any of them).
> - Finally, if this is a C/C++ language feature, you plumb it into Clang to emit the tag when appropriate, with a test that uses -emit-llvm and verify that the IR comes out right.  I'm not aware that "immutable" is a C/C++ feature, but I find it hard to keep up with the C++ standard.
>
> If the whole thing is trivial enough--which it might be, because this is a type qualifier like many others--you can do multiple phases in one patch, but we still want to see that each step has the proper testing.

I did separate patches since they are independent of each other. As a new LLVM contributor, there are some things I miss, and I appreciate this input!

One of the rationales to do this was: since I've seen separate people working in LLVM core and LLDB I decided to separate documentation, and those pieces apart. I thought this could be beneficial for separate reviewers to analyze the change "in the lldb part", "in the llvm core part" and not as a whole. Plus, I noticed that different people get pinged depending on the patch and I assume it has something to do with the patch prefix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113633/new/

https://reviews.llvm.org/D113633



More information about the llvm-commits mailing list