aaron.ballman added inline comments.
================ Comment at: clang/include/clang-c/Index.h:365 + */ + int ExcludeDeclarationsFromPCH : 1; + /** ---------------- vedgy wrote: > aaron.ballman wrote: > > vedgy wrote: > > > vedgy wrote: > > > > aaron.ballman wrote: > > > > > vedgy wrote: > > > > > > Assigning `true` to `int : 1` bit-fields in C++ code produces a GCC > > > > > > warning: > > > > > > ``` > > > > > > warning: overflow in conversion from ‘int’ to ‘signed char:1’ > > > > > > changes value from ‘1’ to ‘-1’ [-Woverflow] > > > > > > ``` > > > > > > > > > > > > Following a suggestion in a comment to > > > > > > https://github.com/llvm/llvm-project/issues/53253, I replaced this > > > > > > `int` with `unsigned` and the warning disappeared. Same for `int > > > > > > DisplayDiagnostics : 1`. Should this type change be included in the > > > > > > upcoming `StorePreamblesInMemory` revision? > > > > > > Assigning true to int : 1 bit-fields in C++ code produces a GCC > > > > > > warning: > > > > > > > > > > > > `warning: overflow in conversion from ‘int’ to ‘signed char:1’ > > > > > > changes value from ‘1’ to ‘-1’ [-Woverflow]` > > > > > > > > > > Ugh, I forgot that the C standard allows that. (C2x 6.7.2.1p12: "A > > > > > bit-field member is interpreted as having a signed or unsigned > > > > > integer type consisting of the specified number of bits" -- GCC > > > > > decided to turn our `int` into `signed char` which is nice for > > > > > packing data together, but not as nice when it comes to boolean-like > > > > > bit-fields.) > > > > > > > > > > > Should this type change be included in the upcoming > > > > > > StorePreamblesInMemory revision? > > > > > > > > > > It'd probably be the cleanest to fix that separately. Given that it's > > > > > NFC and you don't have commit privileges, I can make the change on > > > > > your behalf and land it today if that's what you'd like. > > > > Or should this change be done in a separate revision, on which the > > > > `StorePreamblesInMemory` would be based? > > > > > > > > I also implemented two other changes to the `struct CXIndexOptions` > > > > (mostly documentation/comments). Should these all be in separate > > > > revisions or combined into one? > > > Yes, I agree that such changes should be in separate commits. But I don't > > > want to burden you with committing them all separately. So if 4 is too > > > much, I can request the commit access for myself. If this burden is not > > > too heavy, I am fine with you making the change on my behalf. > > No worries, this was a trivial one -- I landed it in > > dbde7cc17c3a5b6a35e5ec598ba7eaba6f75d90b, so you should be able to fetch > > and rebase on top of that. > > I also implemented two other changes to the struct CXIndexOptions (mostly > > documentation/comments). > Here they are: D145775, D145783. Thank you, I've landed both of them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143418/new/ https://reviews.llvm.org/D143418 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
