Include additional names of events for debug output.

[ywkaras]

No description provided.

[maskit]

Current style looks easier to read than the macro to me, and it's greppable. Why do we want this?

[ywkaras]

It's more typing to not use the macros. This style makes text-level patterns in the code more apparent, particularly any exceptions to the pattern.

[maskit]

Less typing wins readability and greppability? I'm not so sure about that. And it obviously requires more brain resource to see the text-level patterns, and it would require us to understand the macro when we add new ones and changing the names. Why don't you apply the patterns to the exceptions?

[ywkaras]

Based on the reasons I mentioned I personally find the use of macros here makes it more concise and readable to me.

It's correct that token concatenation prevents finding all use of an identifier using text search. I'm not sure that's such an overriding concern. I would hope some of us have better browsing tools (although I don't). But we have the option of a middle-ground approach, macros to avoid some of the repetition, but no use of token concatenation.

[maskit]

But we have the option of a middle-ground approach, macros to avoid some of the repetition, but no use of token concatenation.

Yeh, that would be a compromise. I'm still not sure I can accept it, but sounds like more acceptable than current one.

Another idea would be using something @SolidWallOfCode made recently. I don't remember which PR it was, but I thought we can use it for DebugNames, and I found something I don't like very much, IIRC.

[maskit]

Found the PR, #4222, and you were suggesting macro approach.

[SolidWallOfCode]

For #4222, the code size difference depends on how many times it is used. For the switch approach, you need a non-trivial amount of code for each enumeration. With Lexicon, you have one implementation and when used it's just a variable initialization. Given the number of different enumerations we have, Lexicon would be by far less overall code. It does require repeating the enumeration values, but the syntax is very C++ standard.

For the YAML schema work, the code is generated so repeating enumerations is a negligible cost.

[ywkaras]

I think perhaps the naysayers should contrast the feeling that the code as it is not hard to maintain, with the reality that it's been allowed to sit and rot (not maintained).

[randall]

[approve ci autest]

[maskit]

What part was not maintained? I'd say it haven't been modified because nobody thought it needs to be modified.

Frankly speaking, adding / modifying two lines is much easier than understanding the macro and following the enforced rules to me. The macro might be great if everybody who maintain this function was familiar with the macro, like Debug() macro. But I'm not going to memorize a macro just for this.

Because the names would not be greppable, less people would notice that they actually need to maintain this function.

[ywkaras]

@maskit See lines 216 - 278 in HttpDebugNames.cc. "unknown event" was being returned for all of these events. grep cannot find missing text.

The macros are all defined (and undefined) within the switch statement where they are used.

Whenever you have to manually repeat the same information when coding, and an error will not be caught by the compiler, there is a risk of an error in the delivered code. So a strategy that avoids a lot of repetition of information will almost certainly reduce errors in the delivered code.

[SolidWallOfCode]

I think @maskit's point is that if he wants to update this, he has to understand the macros and convert from the actual event name he has to the "shortened" form based on the macros. Given how rarely this will be done, I think the least error prone way is cut and paste the full explicit names.

If we were serious about this, we'd have a single YAML file with all this data in it and generate the actual C++ code.

[maskit]

See lines 216 - 278 in HttpDebugNames.cc. "unknown event" was being returned for all of these events. grep cannot find missing text.

At least I haven't needed texts for those and I would only add what I need. grep cannot find missing text, but that part is not addressed even with your macro. You just added everything we have now.

@SolidWallOfCode have already clarified my point with an example of adding case, but it's not the only case. If I had to rename or delete an event, I'd grep the event name to find places I need to edit, and I may do the edit with sed command if there are many places. But this doesn't work if the event name was splitted.

[ywkaras]

This is now dependent on #4596