uweigand added a comment.
As of r361920, the SystemZ build bots are green again. Thanks, Eric!
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D37035/new/
https://reviews.llvm.org/D37035
___
cfe-commits mailing list
* The alignment check can be removed, and I'll do so tomorrow if someone
doesn't beat me to it.
On Tue, May 28, 2019 at 1:54 AM Eric Fiselier wrote:
> The alignment check can just be removed.
>
> On Mon., May 27, 2019, 7:57 p.m. Ulrich Weigand via Phabricator, <
> revi...@reviews.llvm.org>
The alignment check can just be removed.
On Mon., May 27, 2019, 7:57 p.m. Ulrich Weigand via Phabricator, <
revi...@reviews.llvm.org> wrote:
> uweigand added a comment.
>
> Looks like this test is failing on SystemZ since it was added, making all
> our build bots red:
>
>
>
uweigand added a comment.
Looks like this test is failing on SystemZ since it was added, making all our
build bots red:
/home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/CodeGenCXX/builtin_FUNCTION.cpp:9:11:
error: CHECK: expected string not found in input
// CHECK:
This revision was automatically updated to reflect the committed changes.
Closed by commit rC360937: Implement __builtin_LINE() et. al. to support source
location capture. (authored by EricWF, committed by ).
Herald added a subscriber: kristina.
Herald added a project: clang.
Repository:
rC
EricWF updated this revision to Diff 199893.
EricWF marked an inline comment as done.
EricWF added a comment.
Merge with upstream.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D37035/new/
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
EricWF marked 14 inline comments as done.
EricWF added a comment.
Address review comments.
Comment at: lib/AST/ExprConstant.cpp:3370-3371
+
+ assert((!Base || !isa(Base)) &&
+ "Base should have already been transformed into a StringLiteral");
+
rsmith
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.
Some cleanups and simplifications, but LGTM with those applied. Thank you!
Comment at: docs/LanguageExtensions.rst:2335
+
+When the
riccibruno marked an inline comment as done.
riccibruno added a comment.
I have no more remark, but since I am just a new contributor to clang I am sure
people will have more to say about this patch. Thanks !
Comment at: lib/AST/Expr.cpp:2091
+
EricWF marked 18 inline comments as done.
EricWF added a comment.
Mark more review comments as done.
Comment at: lib/AST/Expr.cpp:2026-2027
+ // human readable name.
+ if (IdentifierInfo *II = FD->getDeclName().getAsIdentifierInfo())
+return
EricWF updated this revision to Diff 180597.
EricWF marked 4 inline comments as done.
EricWF added a comment.
Address review comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D37035/new/
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
rsmith added inline comments.
Comment at: include/clang/AST/ASTContext.h:279
+ /// A cache mapping a function declaration to its human-readable function or
+ /// file name.
Comment seems out of date: the key here is a string rather than a function
EricWF marked 13 inline comments as done.
EricWF added a comment.
Address review comments. Updated patch coming shortly.
Comment at: include/clang/AST/Expr.h:4147
+ SourceLocation getBeginLoc() const LLVM_READONLY { return BuiltinLoc; }
+ SourceLocation getEndLoc() const
riccibruno added inline comments.
Comment at: include/clang/AST/Expr.h:4108
+public:
+ enum IdentType { Function, File, Line, Column };
+
`IdentType` -> `IdentKind` ?
Comment at: include/clang/AST/Expr.h:4147
+ SourceLocation getBeginLoc()
EricWF added a comment.
Ping! I would like to land this before the next release.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D37035/new/
https://reviews.llvm.org/D37035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
EricWF updated this revision to Diff 179211.
EricWF added a comment.
Herald added a reviewer: shafik.
Merge with upstream.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D37035/new/
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/ASTContext.h
EricWF updated this revision to Diff 167664.
EricWF added a comment.
Build, cache and substitute the `SourceLocExpr` for a `StringLiteral` during
constant evaluation. This is instead of the changes to `LValueBase`which stored
the string value and type in addition to the original
EricWF marked 10 inline comments as done.
EricWF added inline comments.
Comment at: include/clang/AST/APValue.h:66-73
+LValueBase(const Expr *E, const char *StrVal,
+ const ConstantArrayType *ArrTy);
template
-LValueBase(T P, unsigned I = 0,
EricWF added inline comments.
Comment at: docs/LanguageExtensions.rst:2179
+Clang provides experimental builtins to support C++ standard library
implementation
+of `std::experimental::source_location` as specified in
http://wg21.link/N4600.
+With the exception of
rsmith added a comment.
This is in good shape. Some relatively minor comments (plus there are some
unrelated whitespace changes in a few files); the only nontrivial comment I
have is that I think it'd be cleaner to cache `StringLiteral*`s rather than
adding a new form of `LValueBase` for the
EricWF planned changes to this revision.
EricWF added a comment.
Previously this patch performed and leaked a lot of `StringLiteral` temporaries.
I think I have a better solution for that problem, but this patch isn't it.
Updating patch incoming.
https://reviews.llvm.org/D37035
EricWF marked 10 inline comments as done.
EricWF added inline comments.
Comment at: include/clang/AST/SourceLocExprScope.h:39
+//static_assert(foo() == __LINE__);
+return OldVal == nullptr || isa(DefaultExpr) ||
+ !OldVal->isInSameContext(EvalContext);
rsmith added inline comments.
Comment at: include/clang/AST/SourceLocExprScope.h:39
+//static_assert(foo() == __LINE__);
+return OldVal == nullptr || isa(DefaultExpr) ||
+ !OldVal->isInSameContext(EvalContext);
Do we want the `isa` check
EricWF marked 5 inline comments as done.
EricWF added a comment.
In https://reviews.llvm.org/D37035#1116102, @kunitoki wrote:
> Any news on the status of this ? Would be really nice to have it in.
I think this patch is ready to go; but there is more work to be done, as a
separate patch, to
kunitoki added a comment.
Any news on the status of this ? Would be really nice to have it in.
https://reviews.llvm.org/D37035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
EricWF updated this revision to Diff 149009.
EricWF added a comment.
- Fix merge conflicts.
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/RecursiveASTVisitor.h
EricWF updated this revision to Diff 149007.
EricWF added a comment.
- Merge with upstream.
Ping.
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/RecursiveASTVisitor.h
EricWF updated this revision to Diff 134987.
EricWF added a comment.
Ping.
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/RecursiveASTVisitor.h
include/clang/AST/SourceLocExprScope.h
EricWF updated this revision to Diff 133294.
EricWF added reviewers: aaron.ballman, bogner, majnemer.
EricWF added a comment.
Ping.
- Rebase off trunk.
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
EricWF updated this revision to Diff 129746.
EricWF added a comment.
- Introduce `SourceLocExprScope.h` to help reduce code duplication.
- Merge with upstream.
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
EricWF updated this revision to Diff 115349.
EricWF added a comment.
- Improve test.
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/RecursiveASTVisitor.h
include/clang/AST/Stmt.h
EricWF updated this revision to Diff 115342.
EricWF marked 8 inline comments as done.
EricWF added a comment.
- Cleanup and address the inline comments I added earlier.
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
EricWF added inline comments.
Comment at: docs/LanguageExtensions.rst:2118
+point is the location of the caller. When the builtins appear as part of a
+default member initializer, the invocation point is the location of the
+constructor or aggregate initialization used to create
EricWF updated this revision to Diff 115162.
EricWF added a comment.
- Remove accidentally committed test files.
- Attempt to remove incidental whitespace changes.
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/Decl.h
include/clang/AST/Expr.h
EricWF added inline comments.
Comment at: lib/AST/Expr.cpp:1940
false, false),
-InitExprs(C, initExprs.size()),
-LBraceLoc(lbraceloc), RBraceLoc(rbraceloc), AltForm(nullptr, true)
-{
+ InitExprs(C, initExprs.size()), LBraceLoc(lbraceloc),
+
EricWF added inline comments.
Comment at: test/SemaCXX/loc2.cpp:1
+// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// expected-no-diagnostics
Didn't mean to include this file.
Comment at: test/SemaCXX/test.cpp:1
+struct
EricWF updated this revision to Diff 115161.
EricWF marked an inline comment as done.
EricWF added a comment.
- Reimplement without rewriting the AST and instead during the substitution
during constant expression evaluation and code gen.
I still haven't implemented Richards suggestion to reduce
EricWF marked an inline comment as done.
EricWF added a comment.
In https://reviews.llvm.org/D37035#849574, @rsmith wrote:
> I don't like the model of conditionally rebuilding the default initializer /
> default argument if it contains one of these builtins; it seems more
> heavy-handed than
rsmith added a comment.
I don't like the model of conditionally rebuilding the default initializer /
default argument if it contains one of these builtins; it seems more
heavy-handed than necessary. I'm also not convinced that we need to store the
computed value in the AST.
As an alternative,
EricWF updated this revision to Diff 112268.
EricWF marked 4 inline comments as done.
EricWF added a comment.
- Address inline comments.
- Fix calls to class call operators.
https://reviews.llvm.org/D37035
Files:
docs/LanguageExtensions.rst
include/clang/AST/Decl.h
rsmith added inline comments.
Comment at: test/SemaCXX/source_location.cpp:10-35
+struct source_location {
+private:
+ unsigned int __m_line = 0;
+ unsigned int __m_col = 0;
+ const char *__m_file = nullptr;
+ const char *__m_func = nullptr;
+public:
This
majnemer added inline comments.
Comment at: include/clang/AST/Expr.h:3816
+/// __builtin_LINE(), __builtin_COLUMN(), __builtin_FUNCTION(), or
+/// __BUILTIN_FILE()
+class SourceLocExpr final : public Expr {
__BUILTIN_FILE -> __builtin_FILE
hfinkel added inline comments.
Comment at: include/clang/AST/Expr.h:3814
+/// BuiltinSourceLocExpr - Represents a function call to one of
+/// __builtin_LINE(), __builtin_COLUMN(), __builtin_FUNCTION(), or
BuiltinSourceLocExpr -> SourceLocExpr
EricWF created this revision.
This patch implements the source location builtins `__builtin_LINE(),
`__builtin_FUNCTION()`, `__builtin_FILE()` and `__builtin_COLUMN()`. These
builtins are needed to implement
44 matches
Mail list logo
