Remove alternative token usage

[antkmsft]

CI/CD sometimes fails, and MSVC build now fails on some of our personal machines with the latest updates.
#1042 currently fails due to this.

I think it has to do with the compiler version update.

and shouldn't be used anyway - it is an alternative operator, that is intended to be used on the old systems with 7-bit char that don't have good representation for the & character.
https://en.cppreference.com/w/cpp/language/operator_alternative

I think the reason it worked is that it was available in older MSVC versions, but now you must use some sort of a compiler switch in order to make it available.

Other possible explanation, which I hope is not the case, is that someone (us or 3rd party library) has defined "and" as macro, and now it can be substituted by something random, but I don't think it is the case.

Anyways, this change goes in, we compile successfully everywhere.

[ghost]

Hello @antkmsft!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[ahsonkhan]

Though I agree that this change is good, can we please hold off on merging this until we have isolated and verified the root cause of the #1042 CI failure.

If the use of and along with using vcpkg to install 3.9.1 of the JSON library is causing CI failures, it would be good to understand why first.

I suspect the use of and, alone, is not the reason since CI from master and other PRs is green, even with the use of and already checked-in. @vhvb1989 is trying to isolate the issue, and it might be because of needing to react to breaking changes between 3.8.0 and 3.9.1.

[ahsonkhan]

Otherwise, looks good.

[Jinming-Hu]

I want to understand why we used and for logical and and if it's also used elsewhere.

[antkmsft]

@Jinming-Hu , @katmsft introduced that line months ago, and it was fine until some recent MSVC update.
Per https://en.cppreference.com/w/cpp/language/operator_alternative, and many others, and is absolutely the same as && (just another way to write it down), there is no behavior difference, maybe he didn't think it makes significant difference which one to use (I wouldn't).

[antkmsft]

@ahsonkhan , you asked not to merge, removed the auto-merge label 2 hours ago and later approved this 30 minutes ago. I am not sure if you are done with the investigations, so I am not merging this, please merge this PR if you are done.

[Jinming-Hu]

@Jinming-Hu , @katmsft introduced that line months ago, and it was fine until some recent MSVC update.
Per https://en.cppreference.com/w/cpp/language/operator_alternative, and many others, and is absolutely the same as && (just another way to write it down), there is no behavior difference, maybe he didn't think it makes significant difference which one to use (I wouldn't).

I agree that it wouldn't make a big difference. But I believe it's not a good habit to mix them (sometimes and while sometimes &&). I want to be consistent and obviously && is widely used in C++ world.

[antkmsft]

@Jinming-Hu , I agree, and I see no reason to use it, and it is better for consistency. I wouldn't block anyone's PR due to that, but I would leave my feedback. Even if not for fixing any issues, I see our code base better with this change than without it, so this is also why I want to check this in, but now I'm asking @ahsonkhan to ok if he is done with this investigations, since he requested to hold off this for a moment.
To your other question - I didn't find other usages of and or or in our codebase, this is the only one.

[Jinming-Hu]

To your other question - I didn't find other usages of and or or in our codebase, this is the only one.

That's great!

[ahsonkhan]

you asked not to merge, removed the auto-merge label 2 hours ago and later approved this 30 minutes ago.

Apologies for the mixed signal. I will be more clear next time :)
After Jinming approved the PR, there was nothing blocking the merge anyway, so I approved as well, but I should have added a comment to hold off on merging.

I am not sure if you are done with the investigations, so I am not merging this, please merge this PR if you are done.

I will merge it once the investigation is complete and post an update here. It will likely be tomorrow because I wanted to isolate exactly which JSON version triggers the issue (whether it is 3.9.0 or 3.9.1). Thanks for double checking and waiting.

[ahsonkhan]

@antkmsft merge away :)

[antkmsft]

For the context, @vhvb1989 has located the change, it is nlohmann/json@8d29523, and the issue that explains the situation in detail: nlohmann/json#2089