Allow marking an argument as constant in the simple function API

[zhli1142015]

Fixes #7877

[netlify]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	aa2e3f2
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/6582a4924bb35f00073a7355

[zhli1142015]

Hello @laithsakka and @mbasmanova , could you please help review this PR?
Thanks.

[mbasmanova]

@laithsakka gentle ping

[laithsakka]

thank looks good mostly , just some questions and nits
also would it make sense to make
arg_type<Constant> works by translating it to arg_type

[laithsakka]

why are we changing those?
I see NonDefaultWithArrayInitFunction does handles non constant arrays

    if (second == nullptr) {
      return;
    }

question, what is the effect of marking an argument as constant in the signature.

[zhli1142015]

why are we changing those? I see NonDefaultWithArrayInitFunction does handles non constant arrays

I thought the second argument is only handled in initialize function, so it should be marked as constant argument. But, yes, it can handle non constant arrays, I revert this change.

question, what is the effect of marking an argument as constant in the signature.

Constant argument now is only checked in Fuzzer. If one argument is marked as constant, only constant inputs are generated for it by Fuzzer.
#3995 (comment)

[laithsakka]

this will cause lint errors remove names of unsed variables.

[zhli1142015]

Updated, thanks.

[laithsakka]

ditto

[zhli1142015]

Updated, thanks.

[zhli1142015]

Looks test failure is not related to this change.

278: ../../velox/exec/tests/ExchangeClientTest.cpp:310: Failure
278: Expected: (counter) < (kMaxIters), actual: 18 vs 18
278: [  FAILED  ] ExchangeClientTest.sourceTimeout (18001 ms)

[laithsakka]

@zhli1142015 I think you might have missed my comment about handling
arg_type works by translating it to arg_type

ex: if a user used call(..arg_type<Constant>, to have that work, i think now that would just not compile,
but this can be done in a follow up, i will accept this for now.

[facebook-github-bot]

@laithsakka has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[zhli1142015]

I think you might have missed my comment about handling arg_type works by translating it to arg_type

ex: if a user used call(..arg_type, to have that work, i think now that would just not compile, but this can be done in a follow up, i will accept this for now.

Thanks for reviewing this change and sorry for missing that comment. I want to confirm if I corrrectly understand this problem, do you mean if customers define a custom type named "constant" and use it in simplme function API, we should support it?

[facebook-github-bot]

@laithsakka merged this pull request in 9d7d4e8.

[conbench-facebook]

Conbench analyzed the 1 benchmark run on commit 9d7d4e8a.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.