[llvm-dev] Specify default value for FunctionType::get's isVarArg parameter (original) (raw)

James Y Knight via llvm-dev llvm-dev at lists.llvm.org
Thu Feb 7 13:24:00 PST 2019

Personally, I'd rather to delete the overload lacking "Params", and otherwise leave it as is. Make all callers always specify all three arguments.

", {}, false" is really not a lot of clutter.

On Wed, Feb 6, 2019, 10:08 PM Julian Lettner via llvm-dev < llvm-dev at lists.llvm.org> wrote:

Should we specify a default value (false) for the isVarArg parameter of the FunctionType::get functions to improve ergonomics and reduce visual noise?

Typical usage (good citizens annotate their boolean args): … = FunctionType::get(IRB.getVoidTy(), {IntptrTy, IntptrTy}, /isVarArg=/ false); For all usages that I could easily grep, i.e., single-line usages, true was only used 21 times. So at least mechanically, false seems to be a good default. There are different levels for the proposed change. I also state all the cons that I could think of. Level 0: Add default in declaration/header Cons: No, we want to require users to specify isVarArg, i.e., force them to think about the varargs special case. Enables simplified use in the future and gradual cleanup. Level 1: Adapt call sites Cons: This cleanup is not important enough to warrant the churn. Level 2: Get rid of 2-parameter overload by specifying an additional default in the 3-parameter variant: Params = None This requires at least a cleanup of the 2-parameter overload usages. Cons: In addition, this would force ~16 (2-args-true and 2-args-variable) uses to specify None for the params argument (to “gain access” to the isVarArg, i.e., third positional argument). IMO, this is not a cons. No-parameter var-args functions (no format string!) seem special enough that being explicit about the absence of normal parameters feels okay to me. Level 3/orthogonal bonus level: Special getter for void *(void) function type 77 out of 102 usages of the 2-parameter variant are used to retrieve a void func(void) type! Maybe this warrants a special getter, similar to Type::getInt32Ty, for 1) convenience and 2) avoiding the lookup (eagerly initialize it). We can also have a separate discussion for this since it is orthogonal.

What do you think? Are there other cons I have overlooked? Which level would you like to see? Approximate counts (the regex I used are probably not 100% accurate): +--------+-------+------+----------+ | | false | true | variable | +--------+-------+------+----------+ | 3 args | 338 | 14 | 15 | | 2 args | 86 | 7 | 9 | +--------+-------+------+----------+ APIs we are talking about: FunctionType *FunctionType::get(Type *ReturnType, ArrayRef<Type*> Params, bool isVarArg) { // Lookup; create if not exists } FunctionType *FunctionType::get(Type *Result, bool isVarArg) { return get(Result, None, isVarArg); } Regex: ➤ rg "FunctionType::get(" | wc -l 612 # all, including multiline, which are not included below ➤ rg "FunctionType::get([^,]+,[^,]+,[^,]*)" | wc -l 367 # 3 params ➤ rg "FunctionType::get([^,]+,[^,]*)" | wc -l 102 # 2 params ➤ rg "FunctionType::get([^,]+,[^,]+,[^,]*false[^,]*)" | wc -l 338 # 3 params, false ➤ rg "FunctionType::get([^,]+,[^,]+,[^,]*true[^,]*)" | wc -l 14 # 3 params, true ➤ rg "FunctionType::get([^,]+,[^,]*false[^,]*)" | wc -l 86 # 2 params, false ➤ rg "FunctionType::get([^,]+,[^,]*true[^,]*)" | wc -l 7 # 2 params, true ➤ rg "FunctionType::get([^,][Vv]oid[^,],[^,]*false[^,]*)" | wc -l 77 # void (void) function type

LLVM Developers mailing list llvm-dev at lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190207/270c02f8/attachment.html>

More information about the llvm-dev mailing list