ninja clang clangd
Details
Diff Detail
Event Timeline
clang/include/clang/Basic/Specifiers.h | ||
---|---|---|
111 | This changes the size of the structure. It used to be a two byte struct (needing 11 bits with 5 bits of padding) and now I think it will wind up being a three byte struct (9 bits for bit-fields, 6 bits of padding, and then 8 bits for the enum). Do you know how this impacts memory footprint or build performance? | |
clang/include/clang/Sema/DeclSpec.h | ||
338 | This change also changes the size of the structure, but further, it breaks the allocation unit for the bit-fields because TypeSpecComplex can no longer pack together with SCS_extern_in_linkage_spec due to the intervening non-bit-field. |
clang/include/clang/Basic/Specifiers.h | ||
---|---|---|
40 | I don't think there's a lot of benefit to fixing the underlying type to unsigned char now that we're not using the type in a structure -- should this be left off (so it's int implicitly)? | |
109 | How about: /*TypeSpecifierWidth*/ unsigned Width: 2;? | |
clang/include/clang/Sema/DeclSpec.h | ||
338 | How about: /*TypeSpecifierWidth*/ unsigned Width: 2;? | |
clang/lib/Sema/DeclSpec.cpp | ||
690 | Do we maybe want to use getTypeSpecWidth() instead of all of the static_cast<>s in this file? |
- added comments to members
- removed underlying type
- replace static_casts with getTypeSpecWidth()
clang/lib/Sema/DeclSpec.cpp | ||
---|---|---|
690 | Thanks! |
LGTM aside from a place where I think we can remove a cast.
clang/lib/Sema/DeclSpec.cpp | ||
---|---|---|
1100 | Should this one be TypeSpecWidth since it's already an unsigned? |
Could you please commit this for me as I have no privileges. "Thorsten Schuett <schuett@gmail.com>". Thanks.
clang/lib/Sema/DeclSpec.cpp | ||
---|---|---|
1100 | getTypeSpecWidth() returns a TypeSpecWidth and writttenBS.Width is an unsigned? |
clang/lib/Sema/DeclSpec.cpp | ||
---|---|---|
1100 | Yes, I will change it. |
Committed here: https://github.com/llvm/llvm-project/commit/e6aa06545b123292be283af7c414daead23cf9ab .
Thanks Thorsten - please keep these patches coming :)
I don't think there's a lot of benefit to fixing the underlying type to unsigned char now that we're not using the type in a structure -- should this be left off (so it's int implicitly)?