This revision was automatically updated to reflect the committed changes.
Closed by commit rL334650: Implement constexpr __builtin_*_overflow (authored
by erichkeane, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D48040?vs=151151&id=1
efriedma added a comment.
Oh, sorry, I mixed up the two values. I meant that you should add a couple
testcases to ensure the stored value is correct on overflow.
https://reviews.llvm.org/D48040
___
cfe-commits mailing list
cfe-commits@lists.llvm.o
erichkeane added a comment.
I believe my tests DO validate the return value correctly, don't they? It uses
a sentinel, but the ternary should check that return value, right?
Or is there an obvious thing I'm missing?
https://reviews.llvm.org/D48040
__
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.
I'd like to see a couple of testcases ensuring the return value is correct on
overflow (we had a problem with that for __builtin_mul_overflow in the past).
Otherwise LGTM.
https://review
erichkeane updated this revision to Diff 151151.
erichkeane added a comment.
Separated out the other patch as Eli suggested (which has now been committed),
and rebased this patch on top of it.
https://reviews.llvm.org/D48040
Files:
lib/AST/ExprConstant.cpp
test/SemaCXX/builtins-overflow.cp
rjmccall added a comment.
Thanks, comment change looks good. LGTM.
https://reviews.llvm.org/D48040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane added a comment.
Alright, done here: https://reviews.llvm.org/D48053
This one'll require some rebasing on that change, but I'm not sure how to do it
in SVN. Therefore, I'll just rebase this one when it comes to commit it.
-Erich
https://reviews.llvm.org/D48040
_
erichkeane added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:210
+Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg);
+TheCall->setArg(I, Arg.get());
}
efriedma wrote:
> erichkeane wrote:
> > efriedma wrote:
> > > Can you split
efriedma added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:210
+Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg);
+TheCall->setArg(I, Arg.get());
}
erichkeane wrote:
> efriedma wrote:
> > Can you split this change into a sepa
erichkeane added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:210
+Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg);
+TheCall->setArg(I, Arg.get());
}
efriedma wrote:
> Can you split this change into a separate patch? Testcas
efriedma added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:210
+Arg = S.PerformCopyInitialization(Entity, SourceLocation(), Arg);
+TheCall->setArg(I, Arg.get());
}
Can you split this change into a separate patch? Testcase:
```
int a() {
erichkeane updated this revision to Diff 150808.
erichkeane marked an inline comment as done.
https://reviews.llvm.org/D48040
Files:
lib/AST/ExprConstant.cpp
lib/Sema/SemaChecking.cpp
test/SemaCXX/builtins-overflow.cpp
Index: lib/AST/ExprConstant.cpp
===
rjmccall added inline comments.
Comment at: lib/AST/ExprConstant.cpp:8260
+ // It won't GROW, since that isn't possible, so use this to allow
+ // TruncOrSelf.
+ APSInt Temp = Result.extOrTrunc(Info.Ctx.getTypeSize(ResultType));
The comment should
erichkeane created this revision.
erichkeane added reviewers: eli.friedman, rjmccall.
As requested here:https://bugs.llvm.org/show_bug.cgi?id=37633
permit the __builtin_*_overflow builtins in constexpr functions.
Repository:
rC Clang
https://reviews.llvm.org/D48040
Files:
lib/AST/ExprCons
14 matches
Mail list logo
