[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
* 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> wrote: > >> 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: @[[EMPTY_STR:.+]] = private unnamed_addr constant [1 x i8] >> zeroinitializer, align 1 >> ^ >> >> /home/uweigand/sandbox/buildbot/clang-s390x-linux/stage1/tools/clang/test/CodeGenCXX/Output/builtin_FUNCTION.cpp.tmp.ll:1:1: >> note: scanning from here >> ; ModuleID = >> '/home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/CodeGenCXX/builtin_FUNCTION.cpp' >> ^ >> >> /home/uweigand/sandbox/buildbot/clang-s390x-linux/stage1/tools/clang/test/CodeGenCXX/Output/builtin_FUNCTION.cpp.tmp.ll:8:1: >> note: possible intended match here >> @.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 2 >> ^ >> >> The problem is that string constants are 2-aligned according to the >> SystemZ ABI (this is a bit different, but deliberate in order to allow >> computing their addresses with a LARL instruction). This makes all the >> "align 1" checks fail. >> >> Is this test deliberately verifying the alignment, or could this just be >> removed? >> >> >> Repository: >> rC Clang >> >> CHANGES SINCE LAST ACTION >> https://reviews.llvm.org/D37035/new/ >> >> https://reviews.llvm.org/D37035 >> >> >> >> ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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: > > > /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: @[[EMPTY_STR:.+]] = private unnamed_addr constant [1 x i8] > zeroinitializer, align 1 > ^ > > /home/uweigand/sandbox/buildbot/clang-s390x-linux/stage1/tools/clang/test/CodeGenCXX/Output/builtin_FUNCTION.cpp.tmp.ll:1:1: > note: scanning from here > ; ModuleID = > '/home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/CodeGenCXX/builtin_FUNCTION.cpp' > ^ > > /home/uweigand/sandbox/buildbot/clang-s390x-linux/stage1/tools/clang/test/CodeGenCXX/Output/builtin_FUNCTION.cpp.tmp.ll:8:1: > note: possible intended match here > @.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 2 > ^ > > The problem is that string constants are 2-aligned according to the > SystemZ ABI (this is a bit different, but deliberate in order to allow > computing their addresses with a LARL instruction). This makes all the > "align 1" checks fail. > > Is this test deliberately verifying the alignment, or could this just be > removed? > > > Repository: > rC Clang > > CHANGES SINCE LAST ACTION > https://reviews.llvm.org/D37035/new/ > > https://reviews.llvm.org/D37035 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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: @[[EMPTY_STR:.+]] = private unnamed_addr constant [1 x i8] zeroinitializer, align 1 ^ /home/uweigand/sandbox/buildbot/clang-s390x-linux/stage1/tools/clang/test/CodeGenCXX/Output/builtin_FUNCTION.cpp.tmp.ll:1:1: note: scanning from here ; ModuleID = '/home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/CodeGenCXX/builtin_FUNCTION.cpp' ^ /home/uweigand/sandbox/buildbot/clang-s390x-linux/stage1/tools/clang/test/CodeGenCXX/Output/builtin_FUNCTION.cpp.tmp.ll:8:1: note: possible intended match here @.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 2 ^ The problem is that string constants are 2-aligned according to the SystemZ ABI (this is a bit different, but deliberate in order to allow computing their addresses with a LARL instruction). This makes all the "align 1" checks fail. Is this test deliberately verifying the alignment, or could this just be removed? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/ASTContext.h include/clang/AST/CurrentSourceLocExprScope.h include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprObjC.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin-source-location.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp tools/libclang/CXCursor.cpp Index: lib/CodeGen/CGExpr.cpp === --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -1315,11 +1315,15 @@ return LV; } - case Expr::CXXDefaultArgExprClass: -return EmitLValue(cast(E)->getExpr()); + case Expr::CXXDefaultArgExprClass: { +auto *DAE = cast(E); +CXXDefaultArgExprScope Scope(*this, DAE); +return EmitLValue(DAE->getExpr()); + } case Expr::CXXDefaultInitExprClass: { -CXXDefaultInitExprScope Scope(*this); -return EmitLValue(cast(E)->getExpr()); +auto *DIE = cast(E); +CXXDefaultInitExprScope Scope(*this, DIE); +return EmitLValue(DIE->getExpr()); } case Expr::CXXTypeidExprClass: return EmitCXXTypeidLValue(cast(E)); Index: lib/CodeGen/CGExprScalar.cpp === --- lib/CodeGen/CGExprScalar.cpp +++ lib/CodeGen/CGExprScalar.cpp @@ -16,6 +16,7 @@ #include "CGObjCRuntime.h" #include "CodeGenFunction.h" #include "CodeGenModule.h" +#include "ConstantEmitter.h" #include "TargetInfo.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclObjC.h" @@ -640,12 +641,20 @@ Value *VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E) { return EmitLoadOfLValue(E); } + Value *VisitSourceLocExpr(SourceLocExpr *SLE) { +auto &Ctx = CGF.getContext(); +APValue Evaluated = +SLE->EvaluateInContext(Ctx, CGF.CurSourceLocExprScope.getDefaultExpr()); +return ConstantEmitter(CGF.CGM, &CGF) +.emitAbstract(SLE->getLocation(), Evaluated, SLE->getType()); + } Value *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *DAE) { +CodeGenFunction::CXXDefaultArgExprScope Scope(CGF, DAE); return Visit(DAE->getExpr()); } Value *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *DIE) { -CodeGenFunction::CXXDefaultInitExprScope Scope(CGF); +CodeGenFunction::CXXDefaultInitExprScope Scope(CGF, DIE); return Visit(DIE->getExpr()); } Value *VisitCXXThisExpr(CXXThisExpr *TE) { Index: lib/CodeGen/CGExprConstant.cpp === --- lib/CodeGen/CGExprConstant.cpp +++ lib/CodeGen/CGExprConstant.cpp @@ -884,10 +884,6 @@ llvm_unreachable("Invalid CastKind"); } - llvm::Constant *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *DAE, QualType T) { -return Visit(DAE->getExpr(), T); - } - llvm::Constant *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *DIE, QualType T) { // No need for a DefaultInitExprScope: we don't handle 'this' in a // constant expression. Index: lib/CodeGen/CGExprAgg.cpp === --- lib/CodeGen/CGExprAgg.cpp +++ lib/CodeGen/CGExprAgg.cpp @@ -165,10 +165,11 @@ void VisitImplicitValueInitExpr(ImplicitValueInitExpr *E); void VisitNoInitExpr(NoInitExpr *E) { } // Do nothing. void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *DAE) { +CodeGenFunction::CXXDefaultArgExprScope Scope(CGF, DAE); Visit(DAE->getExpr()); } void VisitCXXDefaultInitExpr(CXXDefaultInitExpr *DIE) { -CodeGenFunction::CXXDefaultInitExprScope Scope(CGF); +Code
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/ASTContext.h include/clang/AST/CurrentSourceLocExprScope.h include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprObjC.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin-source-location.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp tools/libclang/CXCursor.cpp Index: tools/libclang/CXCursor.cpp === --- tools/libclang/CXCursor.cpp +++ tools/libclang/CXCursor.cpp @@ -282,6 +282,7 @@ case Stmt::ParenListExprClass: case Stmt::PredefinedExprClass: case Stmt::ShuffleVectorExprClass: + case Stmt::SourceLocExprClass: case Stmt::ConvertVectorExprClass: case Stmt::VAArgExprClass: case Stmt::ObjCArrayLiteralClass: Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,590 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +template +struct Printer; + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 wrote: > EricWF wrote: > > rsmith wrote: > > > There are lots of forms of expression that cannot be the base of an > > > `LValue` (see the list above `LValueExprEvaluator` for the expression > > > forms that *can* be the base of an `LValue`); is it important to give > > > this one special treatment? > > Because a `SourceLocExpr` *can* be the base of an `LValue`. But the way > > that's supported is by transforming the `SourceLocExpr` into a > > `StringLiteral`. > I don't agree: a `SourceLocExpr` cannot be the base of an `LValue`. It is > evaluated into something else that can be (a `StringLiteral`), but it itself > cannot be (and this is in no way unusual; that's probably true of most `Expr` > nodes). I think this is a remnant of an earlier design? Seems like it. Removed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 builtins appears as part of a default function argument the invocation +point is the location of the caller. When the builtins appear as part of a appears -> appear Comment at: include/clang/AST/CurrentSourceLocExprScope.h:31 +public: + /// A RAII style scope gaurd used for tracking the current source + /// location and context as used by the source location builtins gaurd -> guard Comment at: include/clang/AST/Expr.h:4168 +/// Represents a function call to one of __builtin_LINE(), __builtin_COLUMN(), +/// __builtin_FUNCTION(), or __builtin_FILE() +class SourceLocExpr final : public Expr { Missing period at end of comment. Comment at: lib/AST/ASTContext.cpp:10151-10153 + // Get an array type for the string, according to C99 6.4.5. This includes + // the nul terminator character as well as the string length for pascal + // strings. "the string length for pascal strings" part here seems out of place, since you don't handle that here. Comment at: lib/AST/Expr.cpp:2073 +StringLiteral *Res = Ctx.getPredefinedStringLiteralFromCache(Tmp); +return APValue(Res, CharUnits::Zero(), APValue::NoLValuePath(), 0); + }; Given that the type of the `SourceLocExpr` is `const char*`, I'd expect the `APValue` result to be a decayed pointer to the first character in the string, not a pointer to the string itself. (So I think this should have a path of length 1 containing `LValuePathEntry{.ArrayIndex = 0}`.) Comment at: lib/AST/ExprConstant.cpp:5861-5863 +Result.addArray( +Info, E, +cast(Str->getType()->getAsArrayTypeUnsafe())); (This won't be necessary if/when `EvaluateInContext` returns a pointer to the first character in the string.) Comment at: lib/AST/ExprConstant.cpp:7616 + Info.Ctx, Info.CurrentCall->CurSourceLocExprScope.getDefaultExpr()); + return Success(Evaluated.getInt(), E); +} Consider dropping the `.getInt()` here? (Then this version and the pointer version will be identical, and we could consider moving this to `ExprEvaluatorBase`.) Comment at: lib/AST/ExprConstant.cpp:11286 + case Expr::SourceLocExprClass: // GCC considers the GNU __null value to be an integral constant expression. return NoDiag(); This comment is getting detached from its case label. Maybe just remove it? It seems reasonably obvious that `__null` should be an ICE. Comment at: lib/AST/ExprConstant.cpp:3370-3371 + + assert((!Base || !isa(Base)) && + "Base should have already been transformed into a StringLiteral"); + EricWF wrote: > rsmith wrote: > > There are lots of forms of expression that cannot be the base of an > > `LValue` (see the list above `LValueExprEvaluator` for the expression forms > > that *can* be the base of an `LValue`); is it important to give this one > > special treatment? > Because a `SourceLocExpr` *can* be the base of an `LValue`. But the way > that's supported is by transforming the `SourceLocExpr` into a > `StringLiteral`. I don't agree: a `SourceLocExpr` cannot be the base of an `LValue`. It is evaluated into something else that can be (a `StringLiteral`), but it itself cannot be (and this is in no way unusual; that's probably true of most `Expr` nodes). I think this is a remnant of an earlier design? Comment at: lib/CodeGen/CGExpr.cpp:3241-3281 Address CodeGenFunction::EmitArrayToPointerDecay(const Expr *E, LValueBaseInfo *BaseInfo, TBAAAccessInfo *TBAAInfo) { - assert(E->getType()->isArrayType() && + LValue LV = EmitLValue(E); + return EmitArrayToPointerDecay(E, E->getType(), LV, BaseInfo, TBAAInfo); +} + This change should be unnecessary after changing `EvaluateInContext` to decay the pointer. Comment at: lib/CodeGen/CGExprScalar.cpp:623-634 + +if (SLE->isIntType()) + return Builder.getInt(Evaluated.getInt()); + +// If we're not building an int, then we're building a string literal. +const APValue::LValueBase &Base = Evaluated.getLValueBase(); +const StringLiteral *Str = cast(Base.get()); This can be simplified to: ``` return ConstantEmitter(CGF.CGM, CGF).emitAbstract(SLE->getLocation(), Evaluated, SLE->getType()); ``` (once `EvaluateInContext` decays the pointer). Comment at: lib/Serialization/ASTReaderStmt.cpp:971-977 +void ASTStmtR
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 + : PLoc.getColumn(); +return APValue(IntVal); + } > What's your objection here? That I used int64_t or getIntWidth()? I've > reworked this bit of > code. Let me know if it's still problematic. It was just a very small remark saying that you were doing unsigned -> signed -> unsigned for no apparent reason. Anyway the current way looks fine to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 MkStr(II->getNameStart()); + return MkStr(PredefinedExpr::ComputeName(PredefinedExpr::Function, FD)); rsmith wrote: > Is the benefit of avoiding a `std::string` allocation here really worth the > cost of duplicating this portion of the `__FUNCTION__` logic? > > If we care about the extra string allocation, it'd seem more worthwhile to > extend `ComputeName` so that a stack-allocated `SmallString` buffer can be > passed in (eg, `StringRef ComputeName(..., SmallVectorImpl > &NameStorage)`, where `NameStorage` may, but need not, be used to store the > resulting string). Probably. not. I think this optimization attempt is a remnant from an older caching approach. Comment at: lib/AST/ExprConstant.cpp:2653-2655 +APValue::LValueBase const &Base, uint64_t Index) { + const Expr *Lit = Base.get(); rsmith wrote: > Hm, why the move of the `get` from caller to callee? This widens the set of > possible arguments to `extractStringLiteralCharacter`, but all the new > possibilities result in crashes -- shouldn't the caller, rather than the > callee, still be the place where we house the assertion that the base is > indeed an expression? Remant of an older version of the patch. I'll revert. Sorry, I'll try being better about reviewing patches for unnecessary changes. Comment at: lib/AST/ExprConstant.cpp:3370-3371 + + assert((!Base || !isa(Base)) && + "Base should have already been transformed into a StringLiteral"); + rsmith wrote: > There are lots of forms of expression that cannot be the base of an `LValue` > (see the list above `LValueExprEvaluator` for the expression forms that *can* > be the base of an `LValue`); is it important to give this one special > treatment? Because a `SourceLocExpr` *can* be the base of an `LValue`. But the way that's supported is by transforming the `SourceLocExpr` into a `StringLiteral`. Comment at: lib/CodeGen/CGExprConstant.cpp:1762 +ConstantLValue +ConstantLValueEmitter::VisitSourceLocExpr(const SourceLocExpr *E) { + const APValue::LValueBase& Base = Value.getLValueBase(); rsmith wrote: > Hmm, is this reachable? I thought `SourceLocExpr` was always a prvalue. You're right. Not sure what I was thinking. Comment at: lib/Sema/TreeTransform.h:9990 + bool NeedRebuildFunc = E->getIdentType() == SourceLocExpr::Function && + isa(getSema().CurContext) && + getSema().CurContext != E->getParentContext(); rsmith wrote: > If we generalize `__builtin_FUNCTION()` to behave like `__FUNCTION__` (and > handle function-like contexts that aren't `FunctionDecl`s), this `isa` check > will be wrong. I'll remove it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/ASTContext.h include/clang/AST/CurrentSourceLocExprScope.h include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprObjC.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin-source-location.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp tools/libclang/CXCursor.cpp Index: tools/libclang/CXCursor.cpp === --- tools/libclang/CXCursor.cpp +++ tools/libclang/CXCursor.cpp @@ -279,6 +279,7 @@ case Stmt::ParenListExprClass: case Stmt::PredefinedExprClass: case Stmt::ShuffleVectorExprClass: + case Stmt::SourceLocExprClass: case Stmt::ConvertVectorExprClass: case Stmt::VAArgExprClass: case Stmt::ObjCArrayLiteralClass: Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,590 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +template +struct Printer; + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + const
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 declaration. Comment at: lib/AST/Expr.cpp:2012-2020 + case SourceLocExpr::Function: + case SourceLocExpr::File: { +return [&]() { + auto MkStr = [&](StringRef Tmp) { +StringLiteral *Res = Ctx.getPredefinedStringLiteralFromCache(Tmp); +return APValue(Res, CharUnits::Zero(), APValue::NoLValuePath(), 0); + }; Combining these two cases doesn't really make sense since they have nothing in common (except that both produce strings). Instead, consider hoisting the `MkStr` lambda out of the switch and separating the cases. Comment at: lib/AST/Expr.cpp:2022-2023 + const auto *FD = dyn_cast_or_null(Context); + if (!FD) +return MkStr(""); + // If we have a simple identifier there is no need to cache the Why not call into `PredefinedExpr` for the other cases that `__FUNCTION__` supports (`BlockDecl`, `CapturedDecl`, `ObjCMethodDecl`)? Comment at: lib/AST/Expr.cpp:2026-2027 + // human readable name. + if (IdentifierInfo *II = FD->getDeclName().getAsIdentifierInfo()) +return MkStr(II->getNameStart()); + return MkStr(PredefinedExpr::ComputeName(PredefinedExpr::Function, FD)); Is the benefit of avoiding a `std::string` allocation here really worth the cost of duplicating this portion of the `__FUNCTION__` logic? If we care about the extra string allocation, it'd seem more worthwhile to extend `ComputeName` so that a stack-allocated `SmallString` buffer can be passed in (eg, `StringRef ComputeName(..., SmallVectorImpl &NameStorage)`, where `NameStorage` may, but need not, be used to store the resulting string). Comment at: lib/AST/ExprConstant.cpp:2653-2655 +APValue::LValueBase const &Base, uint64_t Index) { + const Expr *Lit = Base.get(); Hm, why the move of the `get` from caller to callee? This widens the set of possible arguments to `extractStringLiteralCharacter`, but all the new possibilities result in crashes -- shouldn't the caller, rather than the callee, still be the place where we house the assertion that the base is indeed an expression? Comment at: lib/AST/ExprConstant.cpp:3370-3371 + + assert((!Base || !isa(Base)) && + "Base should have already been transformed into a StringLiteral"); + There are lots of forms of expression that cannot be the base of an `LValue` (see the list above `LValueExprEvaluator` for the expression forms that *can* be the base of an `LValue`); is it important to give this one special treatment? Comment at: lib/CodeGen/CGExprConstant.cpp:1762 +ConstantLValue +ConstantLValueEmitter::VisitSourceLocExpr(const SourceLocExpr *E) { + const APValue::LValueBase& Base = Value.getLValueBase(); Hmm, is this reachable? I thought `SourceLocExpr` was always a prvalue. Comment at: lib/CodeGen/CGExprConstant.cpp:1766 + "Expected string literal result"); + const StringLiteral* Str = cast(Base.get()); + `const auto *Str` Comment at: lib/CodeGen/CGExprConstant.cpp:1768-1769 + + // assert(Base.isGlobalString() && + // "source location expression does not evaluate to global string?"); + return CGM.GetAddrOfConstantCString(Str->getBytes()); Please remove or uncomment this. Comment at: lib/Sema/TreeTransform.h:9990 + bool NeedRebuildFunc = E->getIdentType() == SourceLocExpr::Function && + isa(getSema().CurContext) && + getSema().CurContext != E->getParentContext(); If we generalize `__builtin_FUNCTION()` to behave like `__FUNCTION__` (and handle function-like contexts that aren't `FunctionDecl`s), this `isa` check will be wrong. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 LLVM_READONLY { return RParenLoc; } + riccibruno wrote: > I don't think that `LLVM_READONLY` is useful here since it is just a simple > getter. > Same remark for `getIdentType`, `isStringType` and `isIntType`. > And even for `getBuiltinStr` does it actually make a difference ? It probably doesn't make any difference. Removing. Comment at: lib/AST/ASTContext.cpp:10145 + unsigned Length) const { + // A C++ string literal has a const-qualified element type (C++ 2.13.4p1). + EltTy = EltTy.withConst(); riccibruno wrote: > And what about C ? It seems to me that `getStringLiteralArrayType` > should be used systematically when the type of a string literal is needed. > > (eg in `ActOnStringLiteral` but this is not the only place...) It should be. I'll fix it to act like `ActOnStringLiteral` and then deduplicate the code. Comment at: lib/AST/Expr.cpp:1961 +// A C++ string literal has a const-qualified element type (C++ 2.13.4p1). +if (Ctx.getLangOpts().CPlusPlus || Ctx.getLangOpts().ConstStrings) + Ty = Ty.withConst(); riccibruno wrote: > Is it taking into account `adjustStringLiteralBaseType` as in > `getStringLiteralArrayType` ? Nope. Fixed. Comment at: lib/AST/Expr.cpp:2025 + // If we have a simple identifier there is no need to cache the + // human readable name. + if (IdentifierInfo *II = FD->getDeclName().getAsIdentifierInfo()) riccibruno wrote: > This comment is strange since MkStr will call > `getPredefinedStringLiteralFromCache` > when `FD->getDeclName()` is a simple identifier. I think this should say `compute` instead of `cache`. Comment at: lib/AST/Expr.cpp:2037 +llvm::APSInt IntVal( +llvm::APInt(Ctx.getTargetInfo().getIntWidth(), LineOrCol)); +return APValue(IntVal); riccibruno wrote: > Both `getLine`, `getColumn` and `APInt` take an unsigned. What's your objection here? That I used `int64_t` or `getIntWidth()`? I've reworked this bit of code. Let me know if it's still problematic. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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() const LLVM_READONLY { return BuiltinLoc; } + SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; } + I don't think that `LLVM_READONLY` is useful here since it is just a simple getter. Same remark for `getIdentType`, `isStringType` and `isIntType`. And even for `getBuiltinStr` does it actually make a difference ? Comment at: include/clang/AST/Stmt.h:657 + class SourceLocExprBitfields { +friend class SourceLocExpr; Could you please put this in the same place as in `StmtNodes.td` ? (ie just after `PseudoObjectExpr`) At least not under "C++ coroutines TS bitfields classes"... Comment at: lib/AST/ASTContext.cpp:10145 + unsigned Length) const { + // A C++ string literal has a const-qualified element type (C++ 2.13.4p1). + EltTy = EltTy.withConst(); And what about C ? It seems to me that `getStringLiteralArrayType` should be used systematically when the type of a string literal is needed. (eg in `ActOnStringLiteral` but this is not the only place...) Comment at: lib/AST/Expr.cpp:1961 +// A C++ string literal has a const-qualified element type (C++ 2.13.4p1). +if (Ctx.getLangOpts().CPlusPlus || Ctx.getLangOpts().ConstStrings) + Ty = Ty.withConst(); Is it taking into account `adjustStringLiteralBaseType` as in `getStringLiteralArrayType` ? Comment at: lib/AST/Expr.cpp:1978 + BuiltinLoc(BLoc), RParenLoc(RParenLoc), ParentContext(ParentContext) { + SourceLocExprBits.Type = Type; +} The name `Type` here is really unfortunate imho. Comment at: lib/AST/Expr.cpp:1992 + } +} + `llvm_unreachable("unexpected IdentType!")` Comment at: lib/AST/Expr.cpp:2025 + // If we have a simple identifier there is no need to cache the + // human readable name. + if (IdentifierInfo *II = FD->getDeclName().getAsIdentifierInfo()) This comment is strange since MkStr will call `getPredefinedStringLiteralFromCache` when `FD->getDeclName()` is a simple identifier. Comment at: lib/AST/Expr.cpp:2037 +llvm::APSInt IntVal( +llvm::APInt(Ctx.getTargetInfo().getIntWidth(), LineOrCol)); +return APValue(IntVal); Both `getLine`, `getColumn` and `APInt` take an unsigned. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/CurrentSourceLocExprScope.h include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin-source-location.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,590 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +template +struct Printer; + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 `SourceLocExpr`. Currently only `SourceLocExpr` uses the `StringLiteral` cache in `ASTContext`. `PredefinedExpr` could be made to use it at the expense of losing the `SourceLocation` information the `StringLiteral` caches. @rsmith How does this look now? I'll follow up with a separate `__builtin_SOURCE_LOCATION()` patch which returns a single relocatable object containing the file, func, line and column. @rsmith Any pointers on how to implement that? https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/ASTContext.h include/clang/AST/CurrentSourceLocExprScope.h include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTContext.cpp lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin-source-location.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,590 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +template +struct Printer; + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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, unsigned V = 0) -: Ptr(P), CallIndex(I), Version(V) {} +LValueBase(T P, unsigned I = 0, unsigned V = 0) : Ptr(P), NormalData{I, V} { + assert(getBaseKind() == BK_Normal && + "cannot create string base with this constructor"); rsmith wrote: > This seems like a slightly dangerous overload set (eg, what does > `LValueBase(P, 0, 0);` call when `P` is of type `Expr*`?) Removed in place of the string literal caching implementation. Comment at: include/clang/AST/APValue.h:143-146 +union { + NormalLValueData NormalData; + GlobalStringData StrData; +}; rsmith wrote: > This makes `LValueBase` 8 bytes larger (on 64-bit targets) to support an > uncommon case. Does `APValue` get larger too, or is its size dominated by > something else? If needed, we could avoid the size increase by separately > allocating the `GlobalStringData` and storing a pointer to it here -- and > maybe something like a `GlobalStringData*` is what we should be caching on > the `ASTContext` instead of a `const char*`? > > However, as you mentioned in a prior comment: > > > An alternative solution would be to create a `StringLiteral`, cache it in > > `ASTContext` for reuse, and return the new expression. Would this require > > serializing the `ASTContext` cache? Would this be a better solution? > > Yes, I think that'd be a better and cleaner solution. We shouldn't need to > serialize the `ASTContext` cache, because we never serialize `APValue`s. (And > if we ever start doing so, serialization of lvalues denoting `Expr*`s becomes > a hard problem in general, not only in this case.) > > Naturally, you'll need to cache the strings used by `__builtin_FILE` in > addition to the current cache for strings used by `__builtin_FUNCTION`, but > on the plus side you should be able to reuse the existing code that generates > `StringLiteral` objects for `PredefinedExpr`s (and maybe you could even share > a cache between the new builtins and the `PredefinedExpr` handling?) > Yes, I think that'd be a better and cleaner solution. We shouldn't need to > serialize the ASTContext cache, because we never serialize APValues. (And if > we ever start doing so, serialization of lvalues denoting Expr*s becomes a > hard problem in general, not only in this case.) SGTM. I've implemented this instead. > on the plus side you should be able to reuse the existing code that generates > StringLiteral objects for PredefinedExprs (and maybe you could even share a > cache between the new builtins and the PredefinedExpr handling?) The only issue with caching, and in particular adding it to `PredefinedExpr`, is that the `StringLiteral` no longer has meaningful location information. Is this OK? https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 `__builtin_COLUMN`, these builtins are also implemented by rsmith wrote: > rST uses double-backticks for code font. Does this URL get autolinked? Woops. Fixed. Comment at: lib/AST/APValue.cpp:46-49 +template static bool isEmptyOrTombstoneKey(T const &V) { + using DMI = llvm::DenseMapInfo; + return V == DMI::getEmptyKey() || V == DMI::getTombstoneKey(); +} rsmith wrote: > This seems to be unused? It was used, but I folded the definition into that usage. Comment at: lib/AST/ExprConstant.cpp:3353 + APValue Str(LVal.Base, CharUnits::Zero(), APValue::NoLValuePath(), 0); + CompleteObject StrObj(&Str, LVal.Base.getGlobalStringType()->desugar(), +false); rsmith wrote: > Similarly, why do you need to desugar the type here? It does seem redundant. Removing. I think I was using it to generate a QualType from a Type*. Comment at: lib/CodeGen/CGExprConstant.cpp:867-869 - llvm::Constant *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *DAE, QualType T) { -return Visit(DAE->getExpr(), T); - } rsmith wrote: > Why was this removed? I believe it was dead code? Comment at: lib/CodeGen/CGExprConstant.cpp:868-869 llvm::Constant *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *DIE, QualType T) { +// FIXME: Is it ever possible for a SourceLocExpr to be below this node? +// If so, we need to update the CurSourceLocExprScope in CodeGenFunction. + rsmith wrote: > I think the salient point here is: this visitor will fail if it reaches a > `SourceLocExpr`, so there's no need to track state that only it needs. (Same > reason we didn't need a `CXXDefaultInitExprScope` for handling `CXXThisExpr`.) Ack. Removing the comment. https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 case of a global constant string with no associataed `StringLiteral`. 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 `__builtin_COLUMN`, these builtins are also implemented by rST uses double-backticks for code font. Does this URL get autolinked? 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, unsigned V = 0) -: Ptr(P), CallIndex(I), Version(V) {} +LValueBase(T P, unsigned I = 0, unsigned V = 0) : Ptr(P), NormalData{I, V} { + assert(getBaseKind() == BK_Normal && + "cannot create string base with this constructor"); This seems like a slightly dangerous overload set (eg, what does `LValueBase(P, 0, 0);` call when `P` is of type `Expr*`?) Comment at: include/clang/AST/APValue.h:143-146 +union { + NormalLValueData NormalData; + GlobalStringData StrData; +}; This makes `LValueBase` 8 bytes larger (on 64-bit targets) to support an uncommon case. Does `APValue` get larger too, or is its size dominated by something else? If needed, we could avoid the size increase by separately allocating the `GlobalStringData` and storing a pointer to it here -- and maybe something like a `GlobalStringData*` is what we should be caching on the `ASTContext` instead of a `const char*`? However, as you mentioned in a prior comment: > An alternative solution would be to create a `StringLiteral`, cache it in > `ASTContext` for reuse, and return the new expression. Would this require > serializing the `ASTContext` cache? Would this be a better solution? Yes, I think that'd be a better and cleaner solution. We shouldn't need to serialize the `ASTContext` cache, because we never serialize `APValue`s. (And if we ever start doing so, serialization of lvalues denoting `Expr*`s becomes a hard problem in general, not only in this case.) Naturally, you'll need to cache the strings used by `__builtin_FILE` in addition to the current cache for strings used by `__builtin_FUNCTION`, but on the plus side you should be able to reuse the existing code that generates `StringLiteral` objects for `PredefinedExpr`s (and maybe you could even share a cache between the new builtins and the `PredefinedExpr` handling?) Comment at: lib/AST/APValue.cpp:46-49 +template static bool isEmptyOrTombstoneKey(T const &V) { + using DMI = llvm::DenseMapInfo; + return V == DMI::getEmptyKey() || V == DMI::getTombstoneKey(); +} This seems to be unused? Comment at: lib/AST/ASTContext.cpp:9915 +assert(D->getDeclName() && "expected declaration to have a name"); +std::string S = D->getDeclName().getAsString(); +Result = strcpy((char *)Allocate(S.size() + 1), S.c_str()); Please use `PredefinedExpr::ComputeName` here to reuse the same logic that we use for `__FUNCTION__`. We don't want `__FUNCTION__` and `__builtin_FUNCTION()` to return different strings for the same function. Comment at: lib/AST/ExprConstant.cpp:72 +if (B.isGlobalString()) + return B.getGlobalStringType()->desugar(); + Why do you need to desugar the type here? `getType` can generally return a sugared type. Comment at: lib/AST/ExprConstant.cpp:3353 + APValue Str(LVal.Base, CharUnits::Zero(), APValue::NoLValuePath(), 0); + CompleteObject StrObj(&Str, LVal.Base.getGlobalStringType()->desugar(), +false); Similarly, why do you need to desugar the type here? Comment at: lib/CodeGen/CGExprConstant.cpp:867-869 - llvm::Constant *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *DAE, QualType T) { -return Visit(DAE->getExpr(), T); - } Why was this removed? Comment at: lib/CodeGen/CGExprConstant.cpp:868-869 llvm::Constant *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *DIE, QualType T) { +// FIXME: Is it ever possible for a SourceLocExpr to be below this node? +// If so, we need to update the CurSourceLocExprScope in CodeGenFunction. + I think the salient point here is: this visitor will fail if it reaches a `SourceLocExpr`, so there's no need to track state that only it needs. (Same reason we didn't need a `CXXDefaultInitExprScope` for
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 wrote: > Do we want the `isa` check here? I'd think the same problem would occur for > default initializers: > > ``` > struct A { > int n = __builtin_LINE(); > }; > struct B { > A a = {}; > }; > B b = {}; // should give this context as the current one, not the context of > the initializer inside struct B > ``` Good catch. Fixed. Comment at: include/clang/AST/SourceLocExprScope.h:44 +public: + SourceLocExprScopeBase(const Expr *DefaultExpr, SourceLocExprScopeBase **Dest, + EvalContextType *EvalContext) rsmith wrote: > I think it'd be cleaner to add a > > ``` > struct Current { > SourceLocExprScopeBase *Scope; > }; > ``` > > maybe with members to get the location and context for a given > `SourceLocExpr`, and to pass in a `SourceLocExprScopeBase::Current &` here > rather than a `SourceLocExprScopeBase **`. Done. Hopefully I understood the comment. Comment at: include/clang/Sema/Sema.h:4463 + + /// \brief build a potentially resolved SourceLocExpr. + /// rsmith wrote: > Capitalize "build" here. Done and removed `\brief`. Comment at: lib/AST/Expr.cpp:1924-1931 + auto CreateString = [&](StringRef SVal) { +QualType StrTy = BuildSourceLocExprType(Ctx, getIdentType(), +/*IsArray*/ true, SVal.size() + 1); +StringLiteral *Lit = StringLiteral::Create(Ctx, SVal, StringLiteral::Ascii, + /*Pascal*/ false, StrTy, Loc); +assert(Lit && "should not be null"); +return Lit; rsmith wrote: > It doesn't seem reasonable to create a new `StringLiteral` each time a > `SourceLocExpr` is (constant-)evaluated, and seems like it would be unsound > in some cases (we require that reevaluation produces the same result). > > I think the `StringLiteral` here is just for convenience, right? (Instead we > could model the `SourceLocExpr` as being its own kind of array lvalue for the > purpose of constant expression evaluation, like we do for `ObjCEncodeExpr` > for instance.) OK. I think I have a solution for this. Please forgive my poor explanation of the problem. The problem was two fold: First, the type of `SourceLocExpr` represents the decayed type of the underlying array, and not the array type or value-category itself (as `StringLiteral` and `ObjCEncodeExpr` do). Therefore it takes additional work to propagate and ensure that the existing machinery uses the "real array type" of the `SourceLocExpr` and not the type of the `SourceLocExpr` itself. Second,,we need to determine, store, and propagate the type,, value category, and value of a `SourceLocExpr` corresponding to the context in which we first encounter the `SourceLocExpr. None of this contextual information is available in the AST after the fact. A lazy model where `SourceLocExpr` pretends to be a placeholder for the "real array type and value" until said subelements are actually accessed will produce different results than if the `SourceLocExpr` was correctly evaluated in the context it was initially encountered. For Example: ``` const char *cur_file(const char *f = __builtin_FILE()) { return f; } struct TestInit { #line 100 "initial_scope.cpp" const char *my_file = cur_file(); TestInit() {} // FILE should point here. We need to transform/determine the value and type of the `SourceLocExpr` in this context. }; #line 200 "new_scope.cpp" // If we delay evaluating `__builtin_FILE()` until this point, where it's actually needed, it will return `new_scope.cpp`, // because we've already exited the `CXXDefaultInitExpr` scope created when first evaluating `TestInit() = default`. TestInit Default; static_assert(strcmp(Default.my_file, "initial_scope.cpp") == 0); // Fails if lazily evaluated! ``` I worked around this problem by storing the "real array type", the used location, and the used context of the `__builtin_FOO())` expression in the `LValueBase` produced when we first consider the `__builtin_FOO()` expression. Comment at: lib/AST/ExprConstant.cpp:698-704 +/// \brief Source location information about the default argument expression +/// we're evaluating, if any. +SourceLocExprScope *CurCXXDefaultArgScope = nullptr; + +/// \brief Source location information about the default member initializer +/// we're evaluating, if any. +SourceLocExprScope *CurCXXDefaultInitScope = nullptr; rsmith wrote: > Do we really need both of these? It would seem like one 'current' scope would > be sufficient to me. Indeed, we don't seem to need both s
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 here? I'd think the same problem would occur for default initializers: ``` struct A { int n = __builtin_LINE(); }; struct B { A a = {}; }; B b = {}; // should give this context as the current one, not the context of the initializer inside struct B ``` Comment at: include/clang/AST/SourceLocExprScope.h:44 +public: + SourceLocExprScopeBase(const Expr *DefaultExpr, SourceLocExprScopeBase **Dest, + EvalContextType *EvalContext) I think it'd be cleaner to add a ``` struct Current { SourceLocExprScopeBase *Scope; }; ``` maybe with members to get the location and context for a given `SourceLocExpr`, and to pass in a `SourceLocExprScopeBase::Current &` here rather than a `SourceLocExprScopeBase **`. Comment at: include/clang/Sema/Sema.h:4463 + + /// \brief build a potentially resolved SourceLocExpr. + /// Capitalize "build" here. Comment at: include/clang/Serialization/ASTBitCodes.h:1643 + /// A SourceLocExpr record + EXPR_SOURCE_LOC, Missing period. Comment at: lib/AST/Expr.cpp:1924-1931 + auto CreateString = [&](StringRef SVal) { +QualType StrTy = BuildSourceLocExprType(Ctx, getIdentType(), +/*IsArray*/ true, SVal.size() + 1); +StringLiteral *Lit = StringLiteral::Create(Ctx, SVal, StringLiteral::Ascii, + /*Pascal*/ false, StrTy, Loc); +assert(Lit && "should not be null"); +return Lit; It doesn't seem reasonable to create a new `StringLiteral` each time a `SourceLocExpr` is (constant-)evaluated, and seems like it would be unsound in some cases (we require that reevaluation produces the same result). I think the `StringLiteral` here is just for convenience, right? (Instead we could model the `SourceLocExpr` as being its own kind of array lvalue for the purpose of constant expression evaluation, like we do for `ObjCEncodeExpr` for instance.) Comment at: lib/AST/ExprConstant.cpp:698-704 +/// \brief Source location information about the default argument expression +/// we're evaluating, if any. +SourceLocExprScope *CurCXXDefaultArgScope = nullptr; + +/// \brief Source location information about the default member initializer +/// we're evaluating, if any. +SourceLocExprScope *CurCXXDefaultInitScope = nullptr; Do we really need both of these? It would seem like one 'current' scope would be sufficient to me. Comment at: lib/AST/ExprConstant.cpp:7296 +bool IntExprEvaluator::VisitSourceLocExpr(const SourceLocExpr *E) { +assert(E && E->isLineOrColumn()); +llvm::APInt Result; Too much indentation. Comment at: lib/CodeGen/CGExprAgg.cpp:192-199 +void VisitVAArgExpr(VAArgExpr * E); - void EmitInitializationToLValue(Expr *E, LValue Address); - void EmitNullInitializationToLValue(LValue Address); - // case Expr::ChooseExprClass: - void VisitCXXThrowExpr(const CXXThrowExpr *E) { CGF.EmitCXXThrowExpr(E); } - void VisitAtomicExpr(AtomicExpr *E) { -RValue Res = CGF.EmitAtomicExpr(E); -EmitFinalDestCopy(E->getType(), Res); - } +void EmitInitializationToLValue(Expr * E, LValue Address); +void EmitNullInitializationToLValue(LValue Address); +// case Expr::ChooseExprClass: +void VisitCXXThrowExpr(const CXXThrowExpr *E) { CGF.EmitCXXThrowExpr(E); } +void VisitAtomicExpr(AtomicExpr * E) { Something funny happened to the formatting here. Comment at: lib/CodeGen/CGExprScalar.cpp:595 +} else { + StringLiteral *Str = SLE->getStringValue(Ctx, Loc, DC); + return CGF.EmitArrayToPointerDecay(Str).getPointer(); Should we make some attempt to reuse the same string literal for multiple source locations denoting the same file? https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 support `std::source_location` in a better manner as suggested by @rsmith in the 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: rsmith wrote: > This seems suboptimal. It would seem better for the compiler to generate a > global containing the relevant data and to represent a `source_location` as a > pointer to such a value. We should also try to minimize the number of > relocations necessary to build a `source_location` object, since such > constructions are likely to be extremely common in some codebases. We should > also keep in mind that we're likely to want to add fields to > `source_location` in future, so designing it in a way that avoids an ABI > break for such cases would be preferable. > > How long has GCC supported this? @rsmith, I agree with your design for `std::source_location`. However, would it be acceptable for now to commit this patch as only the builtins, since we need those for GCC compatibility anyway? Then the changes for something like `__builtin_source_location()` can be added afterwards? https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/SourceLocExprScope.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,475 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __builtin_LINE()\ +\ + == __LINE__ - 2, ""); + static_assert(\ + _\ +_builtin_LINE() + == __LINE__ - 2, ""); + + return true; +} +// clang-format on +static_assert(test_line_fn()); + +static_assert(__builtin_LINE() == __LINE__, ""); + +constexpr
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/SourceLocExprScope.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,475 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __builtin_LINE()\ +\ + == __LINE__ - 2, ""); + static_assert(\ + _\ +_builtin_LINE() + == __LINE__ - 2, ""); + + return true; +} +// clang-format on +static_assert(test_line_fn()); + +static_assert(__builtin_LINE() == __LINE__, ""); + +co
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,475 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __builtin_LINE()\ +\ + == __LINE__ - 2, ""); + static_assert(\ + _\ +_builtin_LINE() + == __LINE__ - 2, ""); + + return true; +} +// clang-format on +static_assert(test_line_fn()); + +static_assert(__builtin_LINE() == __LINE__, ""); + +constexpr int baz() { retu
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/RecursiveASTVisitor.h include/clang/AST/SourceLocExprScope.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,475 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __builtin_LINE()\ +\ + == __LINE__ - 2, ""); + static_assert(\ + _\ +_builtin_LINE() + == __LINE__ - 2, ""); + + return true; +} +// clang-format on +static_assert(test_line_fn()); + +
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/RecursiveASTVisitor.h include/clang/AST/SourceLocExprScope.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,475 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __builtin_LINE()\ +\ + == __LINE__ - 2, ""); + static_assert(\ + _\ +_builtin_LINE() + == __LINE__ - 2, ""); + + return true; +} +// clang-format on +static_assert(test_line_fn(
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,475 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __builtin_LINE()\ +\ + == __LINE__ - 2, ""); + static_assert(\ + _\ +_builtin_LINE() + == __LINE__ - 2, ""); + + return true; +} +// clang-format on +static_assert(test_line_fn()); + +static_assert(__builtin_LINE() == __LINE__, ""); + +constexpr int baz() { return 101; } + +constexpr int test
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,475 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __builtin_LINE()\ +\ + == __LINE__ - 2, ""); + static_assert(\ + _\ +_builtin_LINE() + == __LINE__ - 2, ""); + + return true; +} +// clang-format on +static_assert(test_line_fn()); + +static_assert(__builtin_L
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 the object. Otherwise FIXME: This texts sucks. I can do better. Comment at: include/clang/AST/Decl.h:2418 FieldDecl(Kind DK, DeclContext *DC, SourceLocation StartLoc, -SourceLocation IdLoc, IdentifierInfo *Id, -QualType T, TypeSourceInfo *TInfo, Expr *BW, bool Mutable, +SourceLocation IdLoc, IdentifierInfo *Id, QualType T, +TypeSourceInfo *TInfo, Expr *BW, bool Mutable, `git clang-format` strikes again... Comment at: lib/AST/Expr.cpp:1895 + switch (getIdentType()) { + default: +llvm_unreachable("should not be here"); Remove default case to make way for diagnostics when new cases are added. Comment at: lib/AST/ExprConstant.cpp:4567 return Error(E); +SourceLocDefaultMemberInitContextRAII Guard(Info, E->getLocStart(), +E->getUsedContext()); `E->getUsedLocation()`? Comment at: lib/AST/ExprConstant.cpp:7134 + Loc = Info.EvaluatingDefaultMemberInit->Loc; +else if (Info.EvaluatingDefaultArg && Info.EvaluatingDefaultArg->isInSameCurrentCall()) + Loc = Info.EvaluatingDefaultArg->Loc; This special control flow should be tested. I think it might be? But not against a NSDMI. Comment at: lib/CodeGen/CodeGenFunction.h:1250 public: +public: + class SourceLocExprScopeBase; redundant `public`. Comment at: lib/CodeGen/CodeGenFunction.h:1253 + + SourceLocExprScopeBase *CurCXXDefaultArgScope = nullptr; + SourceLocExprScopeBase *CurCXXDefaultInitScope = nullptr; These need documentation. Comment at: lib/CodeGen/CodeGenFunction.h:1259 + +static bool ShouldEnable(CodeGenFunction &CGF) { + return CGF.CurCXXDefaultArgScope == nullptr || Needs documentation. Comment at: lib/CodeGen/CodeGenFunction.h:1339 + class CXXDefaultArgExprScope : public SourceLocExprScopeBase { +using Base = SourceLocExprScopeBase; Needs documentation. https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,475 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __builtin_LINE()\ +\ + == __LINE__ - 2, ""); + static_assert(\ + _\ +_builtin_LINE() + == __LINE__ - 2, ""); + + return true; +} +// clang-format on +static_assert(test_line_fn()); + +sta
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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), + RBraceLoc(rbraceloc), AltForm(nullptr, true) { Urg... Not sure how both clang-format and git -w messed up enough to let these whitespace changes through (and the ones below this). Comment at: lib/AST/ExprConstant.cpp:591 +SourceLocContextRAIIBase *EvaluatingDefaultArg; +SourceLocContextRAIIBase *EvaluatingDefaultMemberInit; These members need documentation. Comment at: lib/AST/ExprConstant.cpp:6279 +const auto *DIE = dyn_cast(InitExpr); // Temporarily override This, in case there's a CXXDefaultInitExpr in here. This change seems unnecessary. https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 SL { + static constexpr SL current( Didn't mean to include this file either. https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 the size of `source_location` struct, but I think that would be better done as a separate patch. These builtins still need to be implemented, and to act like their GNU counterparts. I haven't cleaned this patch up yet, but I plan to do so tomorrow. There are going to be silly mistakes until then. https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/Decl.h include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/ASTImporter.cpp lib/AST/Expr.cpp lib/AST/ExprCXX.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Parse/ParseExpr.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/loc2.cpp test/SemaCXX/source_location.cpp test/SemaCXX/test.cpp Index: test/SemaCXX/test.cpp === --- /dev/null +++ test/SemaCXX/test.cpp @@ -0,0 +1,35 @@ +struct SL { + static constexpr SL current( + const char* f = __builtin_FUNCTION(), + unsigned l = __builtin_LINE()) { +return {f, l}; + } + const char* function; + unsigned line; +}; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + + +template +constexpr const char* test_func_template(T, U u = U::current()) { + static_assert(is_equal(U::current().function, __func__)); + static_assert(is_equal(U::current().function, "")); + static_assert(U::current().line== __LINE__); + static_assert(U::current().line== __LINE__); + + return u.function; +} +template +void func_template_tests() { + constexpr auto P = test_func_template(42); +} +template void func_template_tests(); Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,475 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +sta
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 necessary. I'm also not convinced that we need to store the > computed value in the AST. > > As an alternative, `CodeGen` and the expression evaluator can track a current > call context (the innermost call expression or list initialization that is > not within a default argument / default member initializer) consisting of the > enclosing `FunctionDecl` and `SourceLocation`, and when the value of one of > these builtins is needed, pass that context to the AST node to ask what value > it has in that context. This will require a bit more work for the static > analyzer, but should be fairly simple elsewhere and will avoid the extra AST > nodes for the desugared form. That sounds good to me. Could you suggest where said context should correctly live? Comment at: docs/LanguageExtensions.rst:2118 +point is the location of the caller. When the builtins appear as part of a +NSDMI the invocation point is the location of the constructor or +aggregate initialization used to create the object. Otherwise the invocation rsmith wrote: > What is an NSDMI? I think you mean "default member initializer". "Non-static default member initializer". I'll change it to your suggestion. Comment at: include/clang/AST/Expr.h:3838 + /// function. + const char *getBuiltinStr() const LLVM_READONLY; + majnemer wrote: > StringRef? Sure. Why not. I'm never sure how to handle string literals. https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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, `CodeGen` and the expression evaluator can track a current call context (the innermost call expression or list initialization that is not within a default argument / default member initializer) consisting of the enclosing `FunctionDecl` and `SourceLocation`, and when the value of one of these builtins is needed, pass that context to the AST node to ask what value it has in that context. This will require a bit more work for the static analyzer, but should be fairly simple elsewhere and will avoid the extra AST nodes for the desugared form. Comment at: docs/LanguageExtensions.rst:2118 +point is the location of the caller. When the builtins appear as part of a +NSDMI the invocation point is the location of the constructor or +aggregate initialization used to create the object. Otherwise the invocation What is an NSDMI? I think you mean "default member initializer". https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 include/clang/AST/Expr.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/Decl.cpp lib/AST/Expr.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/Parse/ParseExpr.cpp lib/Sema/Sema.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaInit.cpp lib/Sema/SemaOverload.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,472 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __builtin_LINE()\ +\ + == __LINE__ - 2, ""); + static_assert(\ + _\ +_builtin_LINE() + == __LINE__ - 2, ""); + + return true; +} +// clang-format on +static_assert(test_line_fn()); + +static_assert(__builtin_LINE() == __
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 seems suboptimal. It would seem better for the compiler to generate a global containing the relevant data and to represent a `source_location` as a pointer to such a value. We should also try to minimize the number of relocations necessary to build a `source_location` object, since such constructions are likely to be extremely common in some codebases. We should also keep in mind that we're likely to want to add fields to `source_location` in future, so designing it in a way that avoids an ABI break for such cases would be preferable. How long has GCC supported this? https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 Comment at: include/clang/AST/Expr.h:3838 + /// function. + const char *getBuiltinStr() const LLVM_READONLY; + StringRef? https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 Comment at: lib/Sema/SemaExpr.cpp:12955 + SourceMgr.getPresumedLoc(SourceMgr.getExpansionRange(CallerLoc).second); + assert(PLoc.isValid()); // FIXME: Learn how to handle this. + Do you have an example where you don't get back a valid location? Are you going to give back ("", 0, 0) for (file, line, column)? Probably doesn't matter for the function name. Comment at: lib/Sema/SemaInit.cpp:563 + if (SemaRef.CheckCXXDefaultInitExpr(Loc, Field)) { +/* nothing todo */ + } else if (SourceLocExpr::containsSourceLocExpr( // Nothing to do. https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.
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 [`std::experimental::source_location`](https://rawgit.com/cplusplus/fundamentals-ts/v2/main.html#reflection.src_loc.creation). With the exception of `__builtin_COLUMN`, GCC also implements these builtins, and Clangs behavior is intended to match as closely as possible. https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang/Basic/StmtNodes.td include/clang/Basic/TokenKinds.def include/clang/Sema/Sema.h include/clang/Serialization/ASTBitCodes.h lib/AST/Expr.cpp lib/AST/ExprClassification.cpp lib/AST/ExprConstant.cpp lib/AST/ItaniumMangle.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/CodeGen/CGExprAgg.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/Parse/ParseExpr.cpp lib/Sema/Sema.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaInit.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/CodeGenCXX/builtin_FUNCTION.cpp test/CodeGenCXX/builtin_LINE.cpp test/CodeGenCXX/debug-info-line.cpp test/Parser/builtin_source_location.c test/Sema/source_location.c test/SemaCXX/Inputs/source-location-file.h test/SemaCXX/source_location.cpp Index: test/SemaCXX/source_location.cpp === --- /dev/null +++ test/SemaCXX/source_location.cpp @@ -0,0 +1,425 @@ +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics + +#define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42) +#define CURRENT_FROM_MACRO() SL::current() +#define FORWARD(...) __VA_ARGS__ + +namespace std { +namespace experimental { +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: + static constexpr source_location current( + const char *__file = __builtin_FILE(), + const char *__func = __builtin_FUNCTION(), + unsigned int __line = __builtin_LINE(), + unsigned int __col = __builtin_COLUMN()) noexcept { +source_location __loc; +__loc.__m_line = __line; +__loc.__m_col = __col; +__loc.__m_file = __file; +__loc.__m_func = __func; +return __loc; + } + constexpr source_location() = default; + constexpr source_location(source_location const &) = default; + constexpr unsigned int line() const noexcept { return __m_line; } + constexpr unsigned int column() const noexcept { return __m_col; } + constexpr const char *file() const noexcept { return __m_file; } + constexpr const char *function() const noexcept { return __m_func; } +}; +} // namespace experimental +} // namespace std + +using SL = std::experimental::source_location; + +#include "Inputs/source-location-file.h" +namespace SLF = source_location_file; + +constexpr bool is_equal(const char *LHS, const char *RHS) { + while (*LHS != 0 && *RHS != 0) { +if (*LHS != *RHS) + return false; +++LHS; +++RHS; + } + return *LHS == 0 && *RHS == 0; +} + +template +constexpr T identity(T t) { + return t; +} + +template +struct Pair { + T first; + U second; +}; + +template +constexpr bool is_same = false; +template +constexpr bool is_same = true; + +// test types +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); +static_assert(is_same); + +// test noexcept +static_assert(noexcept(__builtin_LINE())); +static_assert(noexcept(__builtin_COLUMN())); +static_assert(noexcept(__builtin_FILE())); +static_assert(noexcept(__builtin_FUNCTION())); + +//===--===// +//__builtin_LINE() +//===--===// + +namespace test_line { + +static_assert(SL::current().line() == __LINE__); +static_assert(SL::current().line() == CURRENT_FROM_MACRO().line()); + +static constexpr SL GlobalS = SL::current(); + +static_assert(GlobalS.line() == __LINE__ - 2); + +// clang-format off +constexpr bool test_line_fn() { + constexpr SL S = SL::current(); + static_assert(S.line() == (__LINE__ - 1), ""); + // The start of the call expression to `current()` begins at the token `SL` + constexpr int ExpectLine = __LINE__ + 3; + constexpr SL S2 + = + SL // Call expression starts here + :: + current + ( + + ) + ; + static_assert(S2.line() == ExpectLine, ""); + + static_assert( + FORWARD( + __builtin_LINE +( +) + ) +== __LINE__ - 1, ""); + static_assert(\ +\ + __buil