subject:"\[PATCH\] D37035\: Implement __builtin_LINE\(\) et. al. to support source location capture."

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2019-05-29 Thread Ulrich Weigand via Phabricator via cfe-commits

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.

2019-05-28 Thread Eric Fiselier via cfe-commits

* 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.

2019-05-27 Thread Eric Fiselier via cfe-commits

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.

2019-05-27 Thread Ulrich Weigand via Phabricator via cfe-commits

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.

2019-05-16 Thread Eric Fiselier via Phabricator via cfe-commits

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  = CGF.getContext();
+APValue Evaluated =
+SLE->EvaluateInContext(Ctx, CGF.CurSourceLocExprScope.getDefaultExpr());
+return ConstantEmitter(CGF.CGM, )
+.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);
+

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2019-05-16 Thread Eric Fiselier via Phabricator via cfe-commits

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() {
+

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2019-05-16 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2019-04-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

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  = 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

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2019-01-08 Thread Bruno Ricci via Phabricator via cfe-commits

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.

2019-01-07 Thread Eric Fiselier via Phabricator via cfe-commits

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 
> )`, 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 ,
 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.

2019-01-07 Thread Eric Fiselier via Phabricator via cfe-commits

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() {
+

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2019-01-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

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 
)`, where `NameStorage` may, but need not, be used to store the 
resulting string).



Comment at: lib/AST/ExprConstant.cpp:2653-2655
+APValue::LValueBase const ,
 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.

2019-01-07 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2019-01-06 Thread Bruno Ricci via Phabricator via cfe-commits

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.

2019-01-05 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2018-12-20 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2018-09-30 Thread Eric Fiselier via Phabricator via cfe-commits

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);
+
+//

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2018-09-30 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2018-09-30 Thread Eric Fiselier via Phabricator via cfe-commits

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 ) {
+  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(, 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.

2018-08-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

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 ) {
+  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(, 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.

2018-06-30 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2018-06-27 Thread Eric Fiselier via Phabricator via cfe-commits

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

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2018-06-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

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.

2018-05-30 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2018-05-30 Thread Lucio Asnaghi via Phabricator via cfe-commits

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.

2018-05-29 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF updated this revision to Diff 149009.
EricWF added a comment.

- Fix merge conflicts.


https://reviews.llvm.org/D37035

Files:
  docs/LanguageExtensions.rst
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  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__, "");
+

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2018-05-29 Thread Eric Fiselier via Phabricator via cfe-commits

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__, "");
+

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2018-02-19 Thread Eric Fiselier via Phabricator via cfe-commits

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()
+  ==

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2018-02-07 Thread Eric Fiselier via Phabricator via cfe-commits

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()\
+\
+  ==

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2018-01-12 Thread Eric Fiselier via Phabricator via cfe-commits

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()\
+\
+

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2017-09-14 Thread Eric Fiselier via Phabricator via cfe-commits

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

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2017-09-14 Thread Eric Fiselier via Phabricator via cfe-commits

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, "");
+

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2017-09-14 Thread Eric Fiselier via Phabricator via cfe-commits

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 ) {
+  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.

2017-09-13 Thread Eric Fiselier via Phabricator via cfe-commits

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__

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2017-09-13 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2017-09-13 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2017-09-13 Thread Eric Fiselier via Phabricator via cfe-commits

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

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2017-08-22 Thread Eric Fiselier via Phabricator via cfe-commits

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.

2017-08-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

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.

2017-08-22 Thread Eric Fiselier via Phabricator via cfe-commits

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(\
+

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2017-08-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

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.

2017-08-22 Thread David Majnemer via Phabricator via cfe-commits

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.

2017-08-22 Thread Hal Finkel via Phabricator via cfe-commits

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.

2017-08-22 Thread Eric Fiselier via Phabricator via cfe-commits

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,

44 matches

Mail list logo