I probably need to add more tests, and maybe need to improve some things. Still, I wanted to already send this out to get feedback on the general approach here.
The RFC with more discussion is here:
nit: Drop trivial braces here.
You shouldn't need vector here, can you not use ArrayRef directly, which supports creation from an initializer list? You might need to explicitly type that values as Attribute.
I think you also need to check more here, in particular that the two are the same type to form a valid complex?
Are the changes around standard constant exercised in a test?
If you don't intend to support anything else than a complex, more invariants checking is necessary.
Can you replace auto with the types here? (except the first one)
Thanks for the review comments :) And sorry for taking so long to respond. I was busy with some other tasks.
I had looked around but didn't find tests for valid constants. I have now added tests for complex constants to Standard/ops.mlir. There were already tests for invalid constants in Standard/invalid.mlir, and I added additional tests to cover the new logic.
Thanks for the suggestion :) Done.
So if I understand correctly, most of the checks should be done in the Verifier. Is there another one except LLVM::ConstantOp verifier that I would have to adapt?
Yes you're right, the verifier will be enough.
However the verifier checks an MLIR type:
What is this catching/exercising by the way?
I have added this to be able to return an error for nested structs.
Good point, I didn't realize that I need to check for LLVMStructType in the verifier. I think I understand a bit better now how the interaction of the verifier and ModuleTranslation is. I hope the code I added makes now more sense :)
Yes, that should be an error. But this logic is now also part of the verifier.