This is an archive of the discontinued LLVM Phabricator instance.

Fix pretty printing the unspecified param of a variadic function
ClosedPublic

Authored by asmith on Jan 5 2018, 7:53 PM.

Details

Summary
  • Fix a bug in PrettyBuiltinDumper that returns "void" as the name for an unspecified builtin type. Since the unspecified param of a variadic function is considered a builtin of unspecified type in PDBs, we set "..." for its name.
    • Provide a method to determine if a PDBSymbolFunc is variadic in PrettyFunctionDumper since PDBSymbolFunc::getArgument() doesn't return the last unspecified-type param.
    • Add a pretty-func-dumper.test to test pretty dumping of variadic functions.

Diff Detail

Repository
rL LLVM

Event Timeline

asmith created this revision.Jan 5 2018, 7:53 PM

Thanks for finding this and adding a test. Only one comment, but after that this will be good.

tools/llvm-pdbutil/PrettyFunctionDumper.cpp
193–194 ↗(On Diff #128841)

This check is pretty unintuitive. Is it possible to implement just by looking at the last argument and checking if it's a builtin with type unspecified? Something like

auto Last = SigArguments->getLast();
if (Last->getType() == PDB_Type::Builtin && Last->getBuiltinType() == PDB_BuiltinType::None)

? Either way, to make sure nobody else has to rediscover this, it would be nice if whatever check we do decide on can be put into a function in PDBFunctionType and PDBFunctionSigType, so that we can just say if (Signature->isCVarArgs()) or if (Func->isCVarArgs())?

asmith updated this revision to Diff 129502.Jan 11 2018, 12:42 PM
asmith edited the summary of this revision. (Show Details)
asmith marked an inline comment as done.

I've added the test and were there any other comments?

Would like to close this review out as it's blocking D41427.

Thanks!

zturner accepted this revision.Jan 16 2018, 3:11 PM

LGTM, sorry for the delay, thanks for reminding me.

This revision is now accepted and ready to land.Jan 16 2018, 3:11 PM
This revision was automatically updated to reflect the committed changes.