Use modern T p = v; notation to initialize class fields #1868
Conversation
kripken
changed the title
kripken
changed the title
| int used, available; // used amount, available amount | ||
| bool alloced; | ||
| int used = 0, | ||
| available = init; // used amount, available amount |
There was a problem hiding this comment.
Is this a standard style we have in the codebase? It looks weird to me to have this split over two lines but only have one int, especially with the comment only on the second line.
There was a problem hiding this comment.
Hmm, I don't think we have a style. I don't feel strongly either way. What does LLVM do, btw?
There was a problem hiding this comment.
Added a commit to do it as you suggested.
| size_t size, used; | ||
| char *buffer = nullptr; | ||
| size_t size = 0, | ||
| used = 0; |
There was a problem hiding this comment.
Same here, I would usually default to putting the multiple declarations on the same line, even with definitions.
There was a problem hiding this comment.
If we are going to use separate lines (which I think is good when adding initialization), my preference would be just to repeat the type on the second line. That reduces the coupling and makes it easy to move things around if we want.
There was a problem hiding this comment.
Yeah, agreed, that's what I added now. It's also nicer to avoid T* x, y confusions with pointers.
There was a problem hiding this comment.
Yeah, that's an even better reason actually.
src/wasm-s-parser.h
Outdated
|
|
||
| size_t line, col; | ||
| size_t line = -1, | ||
| col = -1; |
src/wasm.h
Outdated
| Name name; | ||
| Address initial, max; | ||
| Address initial = 0, | ||
| max = kMaxSize; |
src/wasm.h
Outdated
| Name name; | ||
| Address initial, max; // sizes are in pages | ||
| Address initial = 0, | ||
| max = kMaxSize; // sizes are in pages |
There was a problem hiding this comment.
And here. No worries if this is the standard style, of course.
src/emscripten-optimizer/istring.h
Outdated
| }; | ||
|
|
||
| IString() : str(nullptr) {} | ||
| IString() {} |
There was a problem hiding this comment.
Not sure it makes a big difference, but I generally see = default used for cases like this.
There was a problem hiding this comment.
Thanks, yeah, we should do that too. Adding a commit now.
You had some recent experience with this ;)