[clang] [clang] Assert the enum FPOpts and LangOpts fit into the storage (PR #126166)

Zahira Ammarguellat via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 10 12:30:06 PST 2025


zahiraam wrote:

> > > > Not sure I understand your question? Which assertion? `clang.exe t.c` is triggering an assertion?
> > > 
> > > 
> > > I don't understand why you say
> > > > The value `CX_None` is the defaut value given to the `Range` when no `fcomplex-arithmetic` is used on the command lin
> > > 
> > > 
> > > but imply it does not need to be stored in the bitfield. If you patch this CL, and change the size for `ComplexRange` back, you will see it that it fails the assertion I added if you run the clang test suite.
> > 
> > 
> > OK. I see the issue. Thanks. I think this should be titled [NFC][Clang] YOUR TITLE. If you don't want to make it NFC then you need to add a LIT test :-) Not sure you can make it fail with a LIT test.
> 
> What do you mean? Of course you can't fail the assertion now that the offending code is fixed.

I added your patch to my code base  (added only the assert, didn't change the size of storage for `ComplexRange`) and was able to confirm that the assertions go off. So you are right.
I am agreeing to merge this patch. 

However the rule is that every patch needs at least one LIT test that illustrates the issue and prove that your patch fixes it. You don't have a LIT test in your patch. So, you can either make this patch `NFC` by adding it in your title or you need to produce a LIT test. 

https://github.com/llvm/llvm-project/pull/126166


More information about the cfe-commits mailing list