[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