Details
Diff Detail
Event Timeline
Although nothing wrong with the implementation apart from it complicates a bit the understanding of conventional C parse flow by adding extra corner case, I don't see anything in the spec that states explicitly how the vector components should be parsed. I guess with extra parenthesis the parsing problem can easily be avoided:
((int3)(32, 32, 32)).x
test/Parser/vector-cast-define.cl | ||
---|---|---|
11 | I would prefer to simply the test and avoid using macros... it will be easier for us to maintain it in the future. |
I think this is a good feature for the convenience of user. I've seen usage like this.
I agree. I don't see any reasons why this case doesn't have the right to exist. I don't think that using extra parenthesis is a good solution for solving this problem.
I am just saying that I don't see a big use case for this. I am guessing it can largely come from the macro expansions, but those are generally good style to parenthesize.
Ok. But in current implementation if I forget to parenthesize the defined expression (as in the test) I will get the following message: "error: member reference base type 'int' is not a structure or union". I don't think that the message is clear to understand that you just forgot to add parenthesis.
So, should we change the diagnostic message to do it more understandable or push this patch because it can be more convenience for users?
I don't think that diagnostics can always be very clear. This is not the case neither for C nor C++.
As I said I don't see any issue to continue with this patch. I would just like to see the test simplified a bit.
I would prefer to simply the test and avoid using macros... it will be easier for us to maintain it in the future.