[PATCH] D143418: [libclang] Add API to override preamble storage path

[Igor Kushnir via Phabricator via cfe-commits]

vedgy marked an inline comment as not done.
vedgy added inline comments.


================
Comment at: clang/include/clang-c/Index.h:365
+   */
+  int ExcludeDeclarationsFromPCH : 1;
+  /**
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143418/new/

https://reviews.llvm.org/D143418