[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: lib/Sema/SemaExpr.cpp:8808
+        Context.getTypeSize(pointerType) == 
+            Context.getTypeSize(IExp->getType()))
+      IsGNUIdiom = true;
----------------
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.


https://reviews.llvm.org/D37042