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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to