rjmccall added inline comments.

================
Comment at: lib/Sema/SemaExpr.cpp:8808
+        Context.getTypeSize(pointerType) == 
+            Context.getTypeSize(IExp->getType()))
+      IsGNUIdiom = true;
----------------
efriedma wrote:
> andrew.w.kaylor wrote:
> > efriedma wrote:
> > > rsmith wrote:
> > > > andrew.w.kaylor wrote:
> > > > > efriedma wrote:
> > > > > > Please make sure you use exactly the same check in Sema and CodeGen 
> > > > > > (probably easiest to stick a helper into lib/AST/).
> > > > > That's a good idea, but I'm not really familiar enough with the 
> > > > > structure of clang to be sure I'm not doing it in a ham-fisted way.  
> > > > > Can you clarify?  Are you suggesting adding something like 
> > > > > ASTContext::isPointeeTypeCharSize() and 
> > > > > ASTContext::isIntegerTypePointerSize()?  Or maybe a single 
> > > > > specialized function somewhere that does both checks like 
> > > > > ASTContext::areOperandNullPointerArithmeticCompatible()?
> > > > I would suggest something even more specific, such as 
> > > > `isNullPointerPlusIntegerExtension`
> > > I'm most concerned about the difference between 
> > > "isa<llvm::ConstantPointerNull>(pointer)" and 
> > > "PExp->IgnoreParenCasts()->isNullPointerConstant()"... they're different 
> > > in important ways.
> > > 
> > > I was thinking something like 
> > > "BinaryOperator::isNullPointerArithmeticExtension()"
> > At this point in Sema the BinaryOperator for the addition hasn't been 
> > created yet, right?  So a static function that takes the opcode and 
> > operands?
> I wasn't really thinking about that... but yes, something like that.
I've long thought that it would make sense to store a semantic operation kind 
in BinaryOperator and UnaryOperator instead of treating the operator alone as a 
sufficient discriminator.  Of course the operator is *usually* sufficient, but 
there are cases where that's not true — for example, we could use that 
operation kind to distinguish integer and pointer arithmetic, and maybe even to 
identify which side of a + is the pointer.

If we had that, we could very easily flag a null+int operation as a completely 
different semantic operation, which it basically is.


https://reviews.llvm.org/D37042



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to