• Remove sugar from pointee types

@rsmith

• Checks for sugared types and expressions wrapped in parenthesis

@rsmith any more feedback on this current version? If it still looks incorrect to use the record this way, I don't mind simplifying it to work on lvalue to rvalue conversions without checking for a leading address space operation.

Should there be any tests associated with these changes?

• Removed checks for OpenCL in checkConditionalPointerCompatibility. This allows for the error err_typecheck_op_on_nonoverlapping_address_space_pointers to be dumped on getting pointers with different address spaces instead of the warning ext_typecheck_cond_incompatible_pointers.

• Changed tick to single quote in diagnostic

• Replaced instances of a pointer type mismatch warning involving 2 conditional operands with different address spaces with a new error specifically for this situation.

In D50278#1189919, @rjmccall wrote:

I would expect this to replace the existing warning, not to appear together with it.

In D48661#1189537, @bricci wrote:

Just a nit but could you please add new-lines to your commit messages.

• Fixed Accumum names

• Changed diff such that an error is dumped instead. The code shouldn't compile in the first place since it involves conversion between pointers from different address_spaces.

• Moved the counter and set into ExpressionEvaluationContextRecord

Your current counter-based approach doesn't work very well in the case where we switch to another context while processing an expression (for example, during template instantiation): you'll defer all the diagnostics for the inner construct until the outer construct is complete. Generally global Sema state doesn't work very well for this reason.

You shouldn't be adding your own ExpressionEvaluationContextRecords. What I was suggesting is that you store a list of pending noderef expressions on the record, and diagnose them when the record is popped (if they've not been removed since).

This is what I initially tried but the last record left on the stack, which is what I add them to, wasn't getting popped. While parsing, the stack of records (from what I've observed) has only one record that doesn't get popped via PopExpressionEvaluationContext. I'm still looking into ways to try to do this though.

• Added tests for checking that noderef can only be used for C style pointers and arrays (not usable for references or ObjC pointers)
• Added warnings for dereferencing a noderef expression where we cannot trace back the original declrefexpr.
• Added warning for adding noderef on a type that noderef cannot be attatched to.
• Added warning group.
• Use SmallPtrSet instead of std::unordered_set

@aaron.ballman @rsmith Any more feedback on this patch?

In D49511#1168692, @rsmith wrote:

The way in which you're checking for the problematic cases is unnecessarily expensive. Instead of performing a separate AST traversal, please detect whether you should be producing the warning directly when forming the problematic expressions. (For example, you could store a list of pending "dereference of noderef" expressions in the ExpressionEvaluationContextRecord, remove items from the list if you find they're the operand of a unary address-of operator, and diagnose any remaining items once you've left the evaluation context.)

• Added checks for expressions in statements other than declarations or expression statements

@ebevhan Any followup on the patch/my previous comments?

@erichkeane I created a patch at https://reviews.llvm.org/D49327 with the fix for this and added you as a reviewer.

In D46915#1162038, @erichkeane wrote:

See this bug here: https://bugs.llvm.org/show_bug.cgi?id=38161

This patch doesn't seem to properly consider integers, and ignores octals, which I suspect shouldn't be the case.

• Renamed fixedPointSemantics to FixedPointSemantics and hid the members behind getters

• Added tests
• Moved all conversion logic into convert
• Saturation is checked by checking the bits above the sign bit in the destination type

• Renamed to APFixedPoint
• Added FixedPointSemantics to represent saturation and whether or not padding is involved. Similar to APFloatSemantics, this indicated how the underlying APSInt passed to this will be used (ie. is the MSB padding or not).

Oh, having the same number of fractional bits is what leads to unsigned types having one bit of padding, and vice versa.

Formatting and forgot to get rid of Group<f_Group> and Flags<CC1Option>

• Add test case for fix where 0.0r would resolve as -1 when creating a fixed point literal.

The logic in this patch is moved to https://reviews.llvm.org/D48456