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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to