[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcaeb56503ec8: [clang][cli] Convert Analyzer option string based options to new option parsing… (authored by jansvoboda11). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84186/new/ https://reviews.llvm.org/D84186 Files: clang/include/clang/Driver/Options.td clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h clang/lib/Frontend/CompilerInvocation.cpp llvm/include/llvm/Option/OptParser.td Index: llvm/include/llvm/Option/OptParser.td === --- llvm/include/llvm/Option/OptParser.td +++ llvm/include/llvm/Option/OptParser.td @@ -161,6 +161,12 @@ code Denormalizer = "denormalizeString"; } +class MarshallingInfoStringInt + : MarshallingInfo { + code Normalizer = "normalizeStringIntegral<"#type#">"; + code Denormalizer = "denormalizeString"; +} + class MarshallingInfoFlag : MarshallingInfo { code Normalizer = "normalizeSimpleFlag"; Index: clang/lib/Frontend/CompilerInvocation.cpp === --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -93,6 +93,7 @@ #include #include #include +#include #include #include @@ -282,12 +283,38 @@ static void denormalizeString(SmallVectorImpl , const char *Spelling, - CompilerInvocation::StringAllocator SA, - unsigned TableIndex, const std::string ) { + CompilerInvocation::StringAllocator SA, unsigned, + Twine Value) { Args.push_back(Spelling); Args.push_back(SA(Value)); } +template ::value && + std::is_constructible::value, + bool> = false> +static void denormalizeString(SmallVectorImpl , + const char *Spelling, + CompilerInvocation::StringAllocator SA, + unsigned TableIndex, T Value) { + denormalizeString(Args, Spelling, SA, TableIndex, Twine(Value)); +} + +template +static Optional normalizeStringIntegral(OptSpecifier Opt, int, + const ArgList , + DiagnosticsEngine ) { + auto *Arg = Args.getLastArg(Opt); + if (!Arg) +return None; + IntTy Res; + if (StringRef(Arg->getValue()).getAsInteger(0, Res)) { +Diags.Report(diag::err_drv_invalid_int_value) +<< Arg->getAsString(Args) << Arg->getValue(); + } + return Res; +} + static Optional normalizeTriple(OptSpecifier Opt, int TableIndex, const ArgList , DiagnosticsEngine ) { @@ -522,14 +549,6 @@ .Case("false", false) .Default(false); - Opts.AnalyzeSpecificFunction = - std::string(Args.getLastArgValue(OPT_analyze_function)); - Opts.maxBlockVisitOnPath = - getLastArgIntValue(Args, OPT_analyzer_max_loop, 4, Diags); - Opts.InlineMaxStackDepth = - getLastArgIntValue(Args, OPT_analyzer_inline_max_stack_depth, - Opts.InlineMaxStackDepth, Diags); - Opts.CheckersAndPackages.clear(); for (const Arg *A : Args.filtered(OPT_analyzer_checker, OPT_analyzer_disable_checker)) { Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h === --- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -259,8 +259,7 @@ bool AnalyzerWerror : 1; /// The inlining stack depth limit. - // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls). - unsigned InlineMaxStackDepth = 5; + unsigned InlineMaxStackDepth; /// The mode of function selection used during inlining. AnalysisInliningMode InliningMode = NoRedundancy; Index: clang/include/clang/Driver/Options.td === --- clang/include/clang/Driver/Options.td +++ clang/include/clang/Driver/Options.td @@ -4110,7 +4110,8 @@ HelpText<"Emit verbose output about the analyzer's progress">, MarshallingInfoFlag<"AnalyzerOpts->AnalyzerDisplayProgress">; def analyze_function : Separate<["-"], "analyze-function">, - HelpText<"Run analysis on specific function (for C++ include parameters in name)">; + HelpText<"Run analysis on specific function (for C++ include parameters in name)">, + MarshallingInfoString<"AnalyzerOpts->AnalyzeSpecificFunction">; def analyze_function_EQ : Joined<["-"], "analyze-function=">, Alias; def trim_egraph : Flag<["-"], "trim-egraph">, HelpText<"Only show error-related paths in the analysis
[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:261-263 /// The inlining stack depth limit. - // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls). - unsigned InlineMaxStackDepth = 5; + unsigned InlineMaxStackDepth; jansvoboda11 wrote: > dexonsmith wrote: > > Hmm, I wonder if this is entirely better. It's not obvious at all, looking > > at the code here, whether `InlineMaxStackDepth` can be used without getting > > initialized at all. > > > > I agree we should have the default value in two places -- I think removing > > assignment to 5 is the right thing to do -- but I'm not sure leaving this > > entirely uninitialized is a good thing. WDYT of initializing it to 0? > I think leaving this uninitialized communicates the fact that it will be > initialized somewhere else. > > If I saw `unsigned InlineMacStackDepth = 0;` and then observed it changing to > `5` without passing `-analyzer-inline-max-stack-depth=5`, I'd be surprised. Okay, that's fair. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84186/new/ https://reviews.llvm.org/D84186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
jansvoboda11 added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:261-263 /// The inlining stack depth limit. - // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls). - unsigned InlineMaxStackDepth = 5; + unsigned InlineMaxStackDepth; dexonsmith wrote: > Hmm, I wonder if this is entirely better. It's not obvious at all, looking at > the code here, whether `InlineMaxStackDepth` can be used without getting > initialized at all. > > I agree we should have the default value in two places -- I think removing > assignment to 5 is the right thing to do -- but I'm not sure leaving this > entirely uninitialized is a good thing. WDYT of initializing it to 0? I think leaving this uninitialized communicates the fact that it will be initialized somewhere else. If I saw `unsigned InlineMacStackDepth = 0;` and then observed it changing to `5` without passing `-analyzer-inline-max-stack-depth=5`, I'd be surprised. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:287 +static void +denormalizeString(SmallVectorImpl , const char *Spelling, + CompilerInvocation::StringAllocator SA, unsigned, T &) { dexonsmith wrote: > jansvoboda11 wrote: > > dexonsmith wrote: > > > jansvoboda11 wrote: > > > > We should keep an eye on the number of instantiations of this function > > > > template (and `normalizeStringIntegral`). > > > > > > > > If it grows, we can employ the SFINAE trick used for > > > > `makeFlagToValueNormalizer`. > > > Does it work to take the parameter as a `Twine` to avoid the template? > > > ``` > > > static void > > > denormalizeString(SmallVectorImpl , const char > > > *Spelling, > > > CompilerInvocation::StringAllocator SA, unsigned, Twine > > > Value) { > > > Args.push_back(Spelling); > > > Args.push_back(SA(Value)); > > > } > > > ``` > > In general no. The `Twine` constructor is `explicit` for some types > > (integers, chars, etc.). > Okay, then I suggest at least handling the ones that are convertible > separately (without the template): > ``` > static void denormalizeString(..., Twine Value) { > Args.push_back(Spelling); > Args.push_back(SA(Value)); > } > > templatestd::enable_if_t::value && >std::is_constructible::value, bool> = > false> > static void denormalizeString(..., T Value) { > denormalizeString(..., Twine(Value)); > } > ``` > That looks good, thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84186/new/ https://reviews.llvm.org/D84186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
jansvoboda11 updated this revision to Diff 312157. jansvoboda11 added a comment. Reduce number of instantiations of denormalizeString template Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84186/new/ https://reviews.llvm.org/D84186 Files: clang/include/clang/Driver/Options.td clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h clang/lib/Frontend/CompilerInvocation.cpp llvm/include/llvm/Option/OptParser.td Index: llvm/include/llvm/Option/OptParser.td === --- llvm/include/llvm/Option/OptParser.td +++ llvm/include/llvm/Option/OptParser.td @@ -161,6 +161,12 @@ code Denormalizer = "denormalizeString"; } +class MarshallingInfoStringInt + : MarshallingInfo { + code Normalizer = "normalizeStringIntegral<"#type#">"; + code Denormalizer = "denormalizeString"; +} + class MarshallingInfoFlag : MarshallingInfo { code Normalizer = "normalizeSimpleFlag"; Index: clang/lib/Frontend/CompilerInvocation.cpp === --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -93,6 +93,7 @@ #include #include #include +#include #include #include @@ -282,12 +283,38 @@ static void denormalizeString(SmallVectorImpl , const char *Spelling, - CompilerInvocation::StringAllocator SA, - unsigned TableIndex, const std::string ) { + CompilerInvocation::StringAllocator SA, unsigned, + Twine Value) { Args.push_back(Spelling); Args.push_back(SA(Value)); } +template ::value && + std::is_constructible::value, + bool> = false> +static void denormalizeString(SmallVectorImpl , + const char *Spelling, + CompilerInvocation::StringAllocator SA, + unsigned TableIndex, T Value) { + denormalizeString(Args, Spelling, SA, TableIndex, Twine(Value)); +} + +template +static Optional normalizeStringIntegral(OptSpecifier Opt, int, + const ArgList , + DiagnosticsEngine ) { + auto *Arg = Args.getLastArg(Opt); + if (!Arg) +return None; + IntTy Res; + if (StringRef(Arg->getValue()).getAsInteger(0, Res)) { +Diags.Report(diag::err_drv_invalid_int_value) +<< Arg->getAsString(Args) << Arg->getValue(); + } + return Res; +} + static Optional normalizeTriple(OptSpecifier Opt, int TableIndex, const ArgList , DiagnosticsEngine ) { @@ -522,14 +549,6 @@ .Case("false", false) .Default(false); - Opts.AnalyzeSpecificFunction = - std::string(Args.getLastArgValue(OPT_analyze_function)); - Opts.maxBlockVisitOnPath = - getLastArgIntValue(Args, OPT_analyzer_max_loop, 4, Diags); - Opts.InlineMaxStackDepth = - getLastArgIntValue(Args, OPT_analyzer_inline_max_stack_depth, - Opts.InlineMaxStackDepth, Diags); - Opts.CheckersAndPackages.clear(); for (const Arg *A : Args.filtered(OPT_analyzer_checker, OPT_analyzer_disable_checker)) { Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h === --- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -259,8 +259,7 @@ bool AnalyzerWerror : 1; /// The inlining stack depth limit. - // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls). - unsigned InlineMaxStackDepth = 5; + unsigned InlineMaxStackDepth; /// The mode of function selection used during inlining. AnalysisInliningMode InliningMode = NoRedundancy; Index: clang/include/clang/Driver/Options.td === --- clang/include/clang/Driver/Options.td +++ clang/include/clang/Driver/Options.td @@ -4110,7 +4110,8 @@ HelpText<"Emit verbose output about the analyzer's progress">, MarshallingInfoFlag<"AnalyzerOpts->AnalyzerDisplayProgress">; def analyze_function : Separate<["-"], "analyze-function">, - HelpText<"Run analysis on specific function (for C++ include parameters in name)">; + HelpText<"Run analysis on specific function (for C++ include parameters in name)">, + MarshallingInfoString<"AnalyzerOpts->AnalyzeSpecificFunction">; def analyze_function_EQ : Joined<["-"], "analyze-function=">, Alias; def trim_egraph : Flag<["-"], "trim-egraph">, HelpText<"Only show error-related paths in the analysis graph">, @@ -4124,7 +4125,9 @@ def analyzer_dump_egraph_EQ : Joined<["-"], "analyzer-dump-egraph=">, Alias; def
[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
dexonsmith added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:261-263 /// The inlining stack depth limit. - // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls). - unsigned InlineMaxStackDepth = 5; + unsigned InlineMaxStackDepth; Hmm, I wonder if this is entirely better. It's not obvious at all, looking at the code here, whether `InlineMaxStackDepth` can be used without getting initialized at all. I agree we should have the default value in two places -- I think removing assignment to 5 is the right thing to do -- but I'm not sure leaving this entirely uninitialized is a good thing. WDYT of initializing it to 0? Comment at: clang/lib/Frontend/CompilerInvocation.cpp:287 +static void +denormalizeString(SmallVectorImpl , const char *Spelling, + CompilerInvocation::StringAllocator SA, unsigned, T &) { jansvoboda11 wrote: > dexonsmith wrote: > > jansvoboda11 wrote: > > > We should keep an eye on the number of instantiations of this function > > > template (and `normalizeStringIntegral`). > > > > > > If it grows, we can employ the SFINAE trick used for > > > `makeFlagToValueNormalizer`. > > Does it work to take the parameter as a `Twine` to avoid the template? > > ``` > > static void > > denormalizeString(SmallVectorImpl , const char *Spelling, > > CompilerInvocation::StringAllocator SA, unsigned, Twine > > Value) { > > Args.push_back(Spelling); > > Args.push_back(SA(Value)); > > } > > ``` > In general no. The `Twine` constructor is `explicit` for some types > (integers, chars, etc.). Okay, then I suggest at least handling the ones that are convertible separately (without the template): ``` static void denormalizeString(..., Twine Value) { Args.push_back(Spelling); Args.push_back(SA(Value)); } template ::value && std::is_constructible::value, bool> = false> static void denormalizeString(..., T Value) { denormalizeString(..., Twine(Value)); } ``` Comment at: clang/lib/Frontend/CompilerInvocation.cpp:289-290 + CompilerInvocation::StringAllocator SA, unsigned, T &) { + static_assert(std::is_constructible::value, +"Cannot convert this value to Twine"); Args.push_back(Spelling); I think it'd be better to reduce the overload set using `std::enable_if` than to add a `static_assert` here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84186/new/ https://reviews.llvm.org/D84186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
jansvoboda11 added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:287 +static void +denormalizeString(SmallVectorImpl , const char *Spelling, + CompilerInvocation::StringAllocator SA, unsigned, T &) { dexonsmith wrote: > jansvoboda11 wrote: > > We should keep an eye on the number of instantiations of this function > > template (and `normalizeStringIntegral`). > > > > If it grows, we can employ the SFINAE trick used for > > `makeFlagToValueNormalizer`. > Does it work to take the parameter as a `Twine` to avoid the template? > ``` > static void > denormalizeString(SmallVectorImpl , const char *Spelling, > CompilerInvocation::StringAllocator SA, unsigned, Twine > Value) { > Args.push_back(Spelling); > Args.push_back(SA(Value)); > } > ``` In general no. The `Twine` constructor is `explicit` for some types (integers, chars, etc.). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84186/new/ https://reviews.llvm.org/D84186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
dexonsmith requested changes to this revision. dexonsmith added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:287 +static void +denormalizeString(SmallVectorImpl , const char *Spelling, + CompilerInvocation::StringAllocator SA, unsigned, T &) { jansvoboda11 wrote: > We should keep an eye on the number of instantiations of this function > template (and `normalizeStringIntegral`). > > If it grows, we can employ the SFINAE trick used for > `makeFlagToValueNormalizer`. Does it work to take the parameter as a `Twine` to avoid the template? ``` static void denormalizeString(SmallVectorImpl , const char *Spelling, CompilerInvocation::StringAllocator SA, unsigned, Twine Value) { Args.push_back(Spelling); Args.push_back(SA(Value)); } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84186/new/ https://reviews.llvm.org/D84186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
jansvoboda11 added a comment. Ready for a review. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:287 +static void +denormalizeString(SmallVectorImpl , const char *Spelling, + CompilerInvocation::StringAllocator SA, unsigned, T &) { We should keep an eye on the number of instantiations of this function template (and `normalizeStringIntegral`). If it grows, we can employ the SFINAE trick used for `makeFlagToValueNormalizer`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84186/new/ https://reviews.llvm.org/D84186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84186: [clang][cli] Convert Analyzer option string based options to new option parsing system
jansvoboda11 updated this revision to Diff 311527. jansvoboda11 added a comment. Rebase, remove `InlineMaxStackDepth` initialization from `AnalyzerOptions.h` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84186/new/ https://reviews.llvm.org/D84186 Files: clang/include/clang/Driver/Options.td clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h clang/lib/Frontend/CompilerInvocation.cpp llvm/include/llvm/Option/OptParser.td Index: llvm/include/llvm/Option/OptParser.td === --- llvm/include/llvm/Option/OptParser.td +++ llvm/include/llvm/Option/OptParser.td @@ -161,6 +161,12 @@ code Denormalizer = "denormalizeString"; } +class MarshallingInfoStringInt + : MarshallingInfo { + code Normalizer = "normalizeStringIntegral<"#type#">"; + code Denormalizer = "denormalizeString"; +} + class MarshallingInfoFlag : MarshallingInfo { code Normalizer = "normalizeSimpleFlag"; Index: clang/lib/Frontend/CompilerInvocation.cpp === --- clang/lib/Frontend/CompilerInvocation.cpp +++ clang/lib/Frontend/CompilerInvocation.cpp @@ -93,6 +93,7 @@ #include #include #include +#include #include #include @@ -281,12 +282,29 @@ return std::string(Arg->getValue()); } -static void denormalizeString(SmallVectorImpl , - const char *Spelling, - CompilerInvocation::StringAllocator SA, - unsigned TableIndex, const std::string ) { +template +static void +denormalizeString(SmallVectorImpl , const char *Spelling, + CompilerInvocation::StringAllocator SA, unsigned, T &) { + static_assert(std::is_constructible::value, +"Cannot convert this value to Twine"); Args.push_back(Spelling); - Args.push_back(SA(Value)); + Args.push_back(SA(Twine(std::forward(Value; +} + +template +static Optional normalizeStringIntegral(OptSpecifier Opt, int, + const ArgList , + DiagnosticsEngine ) { + auto *Arg = Args.getLastArg(Opt); + if (!Arg) +return None; + IntTy Res; + if (StringRef(Arg->getValue()).getAsInteger(0, Res)) { +Diags.Report(diag::err_drv_invalid_int_value) +<< Arg->getAsString(Args) << Arg->getValue(); + } + return Res; } static Optional normalizeTriple(OptSpecifier Opt, int TableIndex, @@ -507,14 +525,6 @@ .Case("false", false) .Default(false); - Opts.AnalyzeSpecificFunction = - std::string(Args.getLastArgValue(OPT_analyze_function)); - Opts.maxBlockVisitOnPath = - getLastArgIntValue(Args, OPT_analyzer_max_loop, 4, Diags); - Opts.InlineMaxStackDepth = - getLastArgIntValue(Args, OPT_analyzer_inline_max_stack_depth, - Opts.InlineMaxStackDepth, Diags); - Opts.CheckersAndPackages.clear(); for (const Arg *A : Args.filtered(OPT_analyzer_checker, OPT_analyzer_disable_checker)) { Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h === --- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -259,8 +259,7 @@ bool AnalyzerWerror : 1; /// The inlining stack depth limit. - // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls). - unsigned InlineMaxStackDepth = 5; + unsigned InlineMaxStackDepth; /// The mode of function selection used during inlining. AnalysisInliningMode InliningMode = NoRedundancy; Index: clang/include/clang/Driver/Options.td === --- clang/include/clang/Driver/Options.td +++ clang/include/clang/Driver/Options.td @@ -3990,7 +3990,8 @@ HelpText<"Emit verbose output about the analyzer's progress">, MarshallingInfoFlag<"AnalyzerOpts->AnalyzerDisplayProgress">; def analyze_function : Separate<["-"], "analyze-function">, - HelpText<"Run analysis on specific function (for C++ include parameters in name)">; + HelpText<"Run analysis on specific function (for C++ include parameters in name)">, + MarshallingInfoString<"AnalyzerOpts->AnalyzeSpecificFunction">; def analyze_function_EQ : Joined<["-"], "analyze-function=">, Alias; def trim_egraph : Flag<["-"], "trim-egraph">, HelpText<"Only show error-related paths in the analysis graph">, @@ -4004,7 +4005,9 @@ def analyzer_dump_egraph_EQ : Joined<["-"], "analyzer-dump-egraph=">, Alias; def analyzer_inline_max_stack_depth : Separate<["-"], "analyzer-inline-max-stack-depth">, - HelpText<"Bound on stack depth while inlining (4 by default)">; + HelpText<"Bound on stack depth while inlining (4 by default)">, + // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls). +