sylvestre.ledru added a comment.
For the record, Firefox was using this trick. This patch is breaking a ci build
(clang trunk + warning as errors)
More information here: https://bugzilla.mozilla.org/show_bug.cgi?id=1402362
Repository:
rL LLVM
https://reviews.llvm.org/D37042
__
efriedma added a comment.
I agree, it doesn't add much value.
Either way, though, please make sure you address the buildbot failures quickly.
Repository:
rL LLVM
https://reviews.llvm.org/D37042
___
cfe-commits mailing list
cfe-commits@lists.llvm
andrew.w.kaylor added a comment.
This is breaking buildbots for 32-bit targets because the offset in 'nullptr +
int8_t_N' is being implicitly cast to an int. That makes the sizeof(offset) ==
sizeof(ptr) check turn out differently than my tests were assuming.
I can get the buildbots green quick
This revision was automatically updated to reflect the committed changes.
Closed by commit rL313666: Teach clang to tolerate the 'p = nullptr + n' idiom
used by glibc (authored by akaylor).
Changed prior to commit:
https://reviews.llvm.org/D37042?vs=115262&id=115889#toc
Repository:
rL LLVM
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D37042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/c
andrew.w.kaylor updated this revision to Diff 115262.
andrew.w.kaylor added a comment.
-Changed GNU idiom from extension to warning.
-Updated to ToT.
https://reviews.llvm.org/D37042
Files:
include/clang/AST/Expr.h
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaK
efriedma added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+ "arithmetic on a null pointer treated as a cast from integer to pointer is a
GNU extension">,
andrew.w.kaylor wrot
andrew.w.kaylor added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+ "arithmetic on a null pointer treated as a cast from integer to pointer is a
GNU extension">,
efriedma wrot
efriedma added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+ "arithmetic on a null pointer treated as a cast from integer to pointer is a
GNU extension">,
andrew.w.kaylor wrot
andrew.w.kaylor added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+ "arithmetic on a null pointer treated as a cast from integer to pointer is a
GNU extension">,
efriedma wrot
efriedma added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6031
InGroup;
+def ext_gnu_null_ptr_arith : Extension<
+ "arithmetic on a null pointer treated as a cast from integer to pointer is a
GNU extension">,
"extension" isn't re
andrew.w.kaylor added a comment.
Does anything else need to be done for this to be ready to land?
https://reviews.llvm.org/D37042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
andrew.w.kaylor added a comment.
Ping.
https://reviews.llvm.org/D37042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
andrew.w.kaylor updated this revision to Diff 113343.
andrew.w.kaylor added a comment.
Fixed value-dependent argument in isNullPointerConstant checks.
Added check for C++ zero offset in subtraction.
Added value-dependent test cases.
https://reviews.llvm.org/D37042
Files:
include/clang/AST/Exp
rsmith added inline comments.
Comment at: lib/AST/Expr.cpp:1857
+ if (!PExp->IgnoreParenCasts()
+ ->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNull))
+return false;
andrew.w.kaylor wrote:
> rsmith wrote:
> > If we get here with a value-dep
andrew.w.kaylor added inline comments.
Comment at: lib/AST/Expr.cpp:1857
+ if (!PExp->IgnoreParenCasts()
+ ->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNull))
+return false;
rsmith wrote:
> If we get here with a value-dependent expression,
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:
> > > rs
rsmith added inline comments.
Comment at: lib/AST/Expr.cpp:1857
+ if (!PExp->IgnoreParenCasts()
+ ->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNull))
+return false;
If we get here with a value-dependent expression, we should treat it as
n
andrew.w.kaylor updated this revision to Diff 113311.
andrew.w.kaylor added a comment.
Refactored the GNU idiom check to be shared by Sema and CodeGen.
Refined the checks so that nullptr+0 doesn't warn in C++.
Added the zero offset qualification in the warning produced with C++.
https://reviews
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:
> > > andr
andrew.w.kaylor added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8808
+Context.getTypeSize(pointerType) ==
+Context.getTypeSize(IExp->getType()))
+ IsGNUIdiom = true;
efriedma wrote:
> rsmith wrote:
> > andrew.w.kaylor wrote:
> >
efriedma added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8808
+Context.getTypeSize(pointerType) ==
+Context.getTypeSize(IExp->getType()))
+ IsGNUIdiom = true;
rsmith wrote:
> andrew.w.kaylor wrote:
> > efriedma wrote:
> > > Plea
rsmith 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:
> > Please make sure you use e
andrew.w.kaylor added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:8808
+Context.getTypeSize(pointerType) ==
+Context.getTypeSize(IExp->getType()))
+ IsGNUIdiom = true;
efriedma wrote:
> Please make sure you use exactly the same ch
andrew.w.kaylor added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6032
+def ext_gnu_null_ptr_arith : Extension<
+ "inttoptr casting using arithmetic on a null pointer is a GNU extension">,
+ InGroup;
rsmith wrote:
> efriedma wrote:
>
rsmith added a comment.
In https://reviews.llvm.org/D37042#855713, @efriedma wrote:
> I didn't think of this earlier, but strictly speaking, I think
> "(char*)nullptr+0" isn't undefined in C++?
Yes, that's correct. (C++'s model is basically equivalent to having an object
of size zero at the n
efriedma added a reviewer: rsmith.
efriedma added a comment.
I didn't think of this earlier, but strictly speaking, I think
"(char*)nullptr+0" isn't undefined in C++? But probably worth emitting the
warning anyway; anyone writing out arithmetic on null is probably doing
something suspicious, e
andrew.w.kaylor updated this revision to Diff 113134.
andrew.w.kaylor added a comment.
Added warnings for null pointer arithmetic.
https://reviews.llvm.org/D37042
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CGExprScalar.cpp
lib/S
hfinkel added a comment.
In https://reviews.llvm.org/D37042#852733, @efriedma wrote:
> It would be nice to warn on any nullptr arithmetic, yes. (We probably want
> the wording of the warning to be a bit different if we're activating this
> workaround.)
+1 (I'll likely want the ability to tur
efriedma added a comment.
It would be nice to warn on any nullptr arithmetic, yes. (We probably want the
wording of the warning to be a bit different if we're activating this
workaround.)
https://reviews.llvm.org/D37042
___
cfe-commits mailing li
andrew.w.kaylor added a comment.
In https://reviews.llvm.org/D37042#850676, @hfinkel wrote:
> I'd really like to see this done in some way such that we can issue a warning
> along with generating well-defined code. The warning can specifically state
> that the code is using an extension, etc.
hfinkel added a comment.
I'd really like to see this done in some way such that we can issue a warning
along with generating well-defined code. The warning can specifically state
that the code is using an extension, etc.
https://reviews.llvm.org/D37042
__
andrew.w.kaylor added a comment.
My intention here was to make this as strict/limited as possible while still
handling the cases of interest. There are two different implementations I want
to handle. You can see the first variation in the __BPTR_ALIGN definition
being added here:
https://git
majnemer added a comment.
In https://reviews.llvm.org/D37042#849726, @majnemer wrote:
> I'd expect this to be more limited.
>
> Something like, "if we have a BinaryOperator of + between a CStyleCastExpr of
> NullToPointer and 'X', emit inttoptr(X)"
Although maybe that is too strict... I guess
majnemer added a comment.
I'd expect this to be more limited.
Something like, "if we have a BinaryOperator of + between a CStyleCastExpr of
NullToPointer and 'X', emit inttoptr(X)"
https://reviews.llvm.org/D37042
___
cfe-commits mailing list
cfe-c
andrew.w.kaylor added a comment.
I'm not sure why the svn attributes got attached to the file I added. I'll
remove them before committing.
https://reviews.llvm.org/D37042
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.or
andrew.w.kaylor created this revision.
This patch adds a hack to clang's emitPointerArithmetic() function to get it to
emit an inttoptr instruction rather than a GEP with a null base pointer when
the 'p = nullptr + n' idiom is used to convert a pointer-sized integer to a
pointer. This idiom is
37 matches
Mail list logo
