On Sat, 20 Jul 2024 11:18:34 GMT, Johan Sjölen <jsjo...@openjdk.org> wrote:
>> src/hotspot/share/prims/wbtestmethods/parserTests.cpp line 132:
>>
>>> 130: } else if (strcmp(type, "FILE") == 0) {
>>> 131: DCmdArgument<FileArgument> *argument =
>>> 132: new DCmdArgument<FileArgument>(name, desc, "FILE", mandatory);
>>
>> Please check indentation.
>
> On top: We hug the `*`s next to the type in Hotspot, not next to the var
> name. So `DCmdArgument<FileArgument>* argument`. This is something to check
> for all new code.
>
> Pre-existing: The indentation of the if-block is wrong.
>
> Also, @SoniaZaldana, would you mind changing the code to this (does not
> include your change), the repetition just made me cringe 😅.
>
> ```c++
> DCmdArgument<char*>* argument = nullptr;
> if (strcmp(type, "STRING") == 0) {
> argument = new DCmdArgument<char*>(name, desc, "STRING", mandatory,
> default_value);
> } else if (strcmp(type, "NANOTIME") == 0) {
> DCmdArgument<NanoTimeArgument>* argument = new
> DCmdArgument<NanoTimeArgument>(name, desc, "NANOTIME", mandatory,
> default_value);
> } else if (strcmp(type, "JLONG") == 0) {
> DCmdArgument<jlong>* argument = new DCmdArgument<jlong>(name, desc,
> "JLONG", mandatory, default_value);
> } else if (strcmp(type, "BOOLEAN") == 0) {
> DCmdArgument<bool>* argument = new DCmdArgument<bool>(name, desc,
> "BOOLEAN", mandatory, default_value);
> } else if (strcmp(type, "MEMORYSIZE") == 0) {
> DCmdArgument<MemorySizeArgument>* argument = new
> DCmdArgument<MemorySizeArgument>(name, desc, "MEMORY SIZE", mandatory,
> default_value);
> } else if (strcmp(type, "STRINGARRAY") == 0) {
> DCmdArgument<StringArrayArgument*>* argument = new
> DCmdArgument<StringArrayArgument*>(name, desc, "STRING SET", mandatory);
> }
>
> if (argument != nullptr) {
> if (isarg) {
> parser->add_dcmd_argument(argument);
> } else {
> parser->add_dcmd_option(argument);
> }
> }
@jdksjolen
> Also, @SoniaZaldana, would you mind changing the code to this
Even simpler (did not test, but you get my drift):
#define ALL_TYPES_DO_XX(what) \
what(char*, "STRING") \
what(NanoTimeArgument, NANOTIME) \
what(jlong, "JLONG")
... etc
then
#define XX(TYPE, NAME) \
if (strcmp(type, NAME) == 0) { \
DCmdArgument<TYPE>* argument = new DCmdArgument<TYPE>(name, desc, NAME,
mandatory, mandatory, default_value); \
}
ALL_TYPES_DO_XX(XX)
#undef XX
;-)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20198#discussion_r1685411741
