Page MenuHomePhabricator

Add debug support for set types
Needs ReviewPublic

Authored by demoitem on Mar 12 2020, 9:12 PM.

Details

Summary

This commit adds debugging support for set types defined in languages such as pascal and modula-2

Diff Detail

Event Timeline

demoitem created this revision.Mar 12 2020, 9:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 12 2020, 9:12 PM
dblaikie added a project: debug-info.
dblaikie accepted this revision.Mar 23 2020, 1:37 PM
dblaikie added a reviewer: aprantl.

Seems reasonable to me - thanks!

This revision is now accepted and ready to land.Mar 23 2020, 1:37 PM

In your example you're encoding set as a derived type over an enum. That seems like a fine approach to me, but I think the interface should enforce this. Perhaps the Verifier should reject set derived typed from anything but an enum and the DIBuilder should only accept DICompositeTypes as the base type? Are there non-enum set types that you also want to encode?
Does this also need dwarfdump support, or does this work already?

demoitem updated this revision to Diff 259159.Apr 21 2020, 9:42 PM

Added extra Verifier checks to ensure the base type of a set is either an Enumeration or a Base type with integer encoding.
I will extend this check when I land a patch to extend subranges since a set can have a subrange as a basetype.
The llvm-dwarfdump tool outputs set debugging information without any enhancements.
I do not have commit access, so I wonder if someone could commit these changes if acceptable?

demoitem requested review of this revision.Apr 21 2020, 9:45 PM

Could someone please commit this patch if acceptable.

Looks like you accidentally just updated the diff on top of your previous patch?
Is the Verifier change being tested?

I'm new to this, my first contribution, so bear with me.

On my local branch I had 2 commits
and I used the following to get a patch (following the docs)

git show HEAD -U999999 > set.patch

The second patch I uploaded seemed to only have the changes from the second commit - which I thought was okay.

Can you advise? Is each patch supposed to contain the complete set of commits for the changed code and get a new Differential number?

I am slightly confused.

Also, regards the Verifier changes, I have only tested them locally but I guess I should produce a test file. So I will get started on that.

Thanks

I'm new to this, my first contribution, so bear with me.

On my local branch I had 2 commits
and I used the following to get a patch (following the docs)

git show HEAD -U999999 > set.patch

The HEAD refers to the SHA of the specific commit.

The second patch I uploaded seemed to only have the changes from the second commit - which I thought was okay.

Can you advise? Is each patch supposed to contain the complete set of commits for the changed code and get a new Differential number?

Yes. If the patches are logically different it makes sense to make separate changes here. If the diffs are connected (they represent the same functionality/feature/intention), it makes sense to make a stack of the patches (Edit Related Revision -> Edit Parent/Child rev).

So, if your local repo looks as:

commit 878787...
Author: ...
Date:   ...

    [LLVM] ..

commit 989898...
Author: ...
Date:   ...

    [Clang] ..

you can create the patches for the Phabricator as:

git show 989898... -U999999 > name_of_the_change1.patch
git show 878787... -U999999 > name_of_the_change2.patch

I am slightly confused.

Also, regards the Verifier changes, I have only tested them locally but I guess I should produce a test file. So I will get started on that.

Thanks