[PATCH] D81131: [DebugInfo] Fix assertion for extern void type

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 4 11:00:24 PDT 2020


dblaikie added a comment.

Patches without tests shouldn't be approved - and at least the usual/rough metric I use for patch approval is (most often - unless I'm reviewing the work of the code owner in any area who wants a second opinion, etc) - I approve it if I'd be willing to commit a similar patch without approval (with post-commit review - ie: I'm fairly sure this is the right direction and folks will agree with me, or minor changes can be handled in post-commit iteration). Otherwise "approval" gets muddied as to whether it means "you can now commit this" versus "I think this is good but you should wait for someone more authoritative to say 'yes' before it's committed" (usually folks on the project call out the latter explicitly - sometimes using Phab's "approve" sometimes not).

In any case: I think if we're going to support "const void" we should support "void" too & fix the verifier to allow this & make sure LLVM produces the correct debug info for it (which is probably a DW_TAG_variable without a DW_AT_type attribute (same as a function with a void return type has no DW_AT_type attribute) - and check that GCC does the same thing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81131





More information about the cfe-commits mailing list