RE: r325081 - Implement function attribute artificial

2018-02-14 Thread Andrews, Elizabeth via cfe-commits 
Thanks Richard. I’ll update the documentation and submit a new patch for review.

Elizabeth

From: Keane, Erich
Sent: Wednesday, February 14, 2018 11:33 AM
To: Richard Smith <rich...@metafoo.co.uk>; Andrews, Elizabeth 
<elizabeth.andr...@intel.com>
Cc: cfe-commits <cfe-commits@lists.llvm.org>
Subject: RE: r325081 - Implement function attribute artificial

Thanks for your comments Richard, I’ve added the author to this email so that 
she can take a look.

From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Tuesday, February 13, 2018 5:39 PM
To: Keane, Erich <erich.ke...@intel.com<mailto:erich.ke...@intel.com>>
Cc: cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r325081 - Implement function attribute artificial

On 13 February 2018 at 16:14, Erich Keane via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Tue Feb 13 16:14:07 2018
New Revision: 325081

URL: http://llvm.org/viewvc/llvm-project?rev=325081=rev
Log:
Implement function attribute artificial

Added support in clang for GCC function attribute 'artificial'. This attribute
is used to control stepping behavior of debugger with respect to inline
functions.

Patch By: Elizabeth Andrews (eandrews)

Differential Revision: https://reviews.llvm.org/D43259


Added:
cfe/trunk/test/CodeGen/artificial.c
cfe/trunk/test/Sema/artificial.c
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=325081=325080=325081=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Feb 13 16:14:07 2018
@@ -111,6 +111,9 @@ def SharedVar : SubsetSubject<Var,
 def GlobalVar : SubsetSubject<Var,
  [{S->hasGlobalStorage()}], "global variables">;

+def InlineFunction : SubsetSubject<Function,
+ [{S->isInlineSpecified()}], "inline functions">;
+
 // FIXME: this hack is needed because DeclNodes.td defines the base Decl node
 // type to be a class, not a definition. This makes it impossible to create an
 // attribute subject which accepts a Decl. Normally, this is not a problem,
@@ -588,6 +591,12 @@ def AlwaysInline : InheritableAttr {
   let Documentation = [Undocumented];
 }

+def Artificial : InheritableAttr {
+  let Spellings = [GCC<"artificial">];
+  let Subjects = SubjectList<[InlineFunction], WarnDiag>;
+  let Documentation = [ArtificialDocs];
+}
+
 def XRayInstrument : InheritableAttr {
   let Spellings = [Clang<"xray_always_instrument">,
Clang<"xray_never_instrument">];

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=325081=325080=325081=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Tue Feb 13 16:14:07 2018
@@ -3273,3 +3273,13 @@ For more information see
 or `msvc documentation <https://docs.microsoft.com/pl-pl/cpp/cpp/selectany>`_.
 }];
 }
+
+def ArtificialDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``artificial`` attribute is used with inline functions to treat the inline
+function as a unit while debugging. For more information see GCC_ 
documentation.

I find this to be hard to understand. What does "treat the inline function as a 
unit" actually mean? How about something like:

The ``artificial`` attribute can be applied to an inline function. If such a 
function is inlined, the attribute indicates that debuggers should associate 
the resulting instructions with the call site, rather than with the 
corresponding line within the inlined callee.

+
+.. _GCC: https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Function-Attributes.html

If you still want to reference the GCC documentation, could you pick a newer 
version? :)

+  }];
+}

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=325081=325080=325081=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Feb 13 16:14:07 2018
@@ -3235,7 +3235,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
   if (Name.startswith("\01"))
 Name = Name.substr(1);

-  if (!HasDecl || D->isImplicit()) {
+  if (!HasDecl || D->isImplicit

RE: r325081 - Implement function attribute artificial

2018-02-14 Thread Keane, Erich via cfe-commits 
Thanks for your comments Richard, I’ve added the author to this email so that 
she can take a look.

From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Tuesday, February 13, 2018 5:39 PM
To: Keane, Erich <erich.ke...@intel.com>
Cc: cfe-commits <cfe-commits@lists.llvm.org>
Subject: Re: r325081 - Implement function attribute artificial

On 13 February 2018 at 16:14, Erich Keane via cfe-commits 
<cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Tue Feb 13 16:14:07 2018
New Revision: 325081

URL: http://llvm.org/viewvc/llvm-project?rev=325081=rev
Log:
Implement function attribute artificial

Added support in clang for GCC function attribute 'artificial'. This attribute
is used to control stepping behavior of debugger with respect to inline
functions.

Patch By: Elizabeth Andrews (eandrews)

Differential Revision: https://reviews.llvm.org/D43259


Added:
cfe/trunk/test/CodeGen/artificial.c
cfe/trunk/test/Sema/artificial.c
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=325081=325080=325081=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Feb 13 16:14:07 2018
@@ -111,6 +111,9 @@ def SharedVar : SubsetSubject<Var,
 def GlobalVar : SubsetSubject<Var,
  [{S->hasGlobalStorage()}], "global variables">;

+def InlineFunction : SubsetSubject<Function,
+ [{S->isInlineSpecified()}], "inline functions">;
+
 // FIXME: this hack is needed because DeclNodes.td defines the base Decl node
 // type to be a class, not a definition. This makes it impossible to create an
 // attribute subject which accepts a Decl. Normally, this is not a problem,
@@ -588,6 +591,12 @@ def AlwaysInline : InheritableAttr {
   let Documentation = [Undocumented];
 }

+def Artificial : InheritableAttr {
+  let Spellings = [GCC<"artificial">];
+  let Subjects = SubjectList<[InlineFunction], WarnDiag>;
+  let Documentation = [ArtificialDocs];
+}
+
 def XRayInstrument : InheritableAttr {
   let Spellings = [Clang<"xray_always_instrument">,
Clang<"xray_never_instrument">];

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=325081=325080=325081=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Tue Feb 13 16:14:07 2018
@@ -3273,3 +3273,13 @@ For more information see
 or `msvc documentation <https://docs.microsoft.com/pl-pl/cpp/cpp/selectany>`_.
 }];
 }
+
+def ArtificialDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``artificial`` attribute is used with inline functions to treat the inline
+function as a unit while debugging. For more information see GCC_ 
documentation.

I find this to be hard to understand. What does "treat the inline function as a 
unit" actually mean? How about something like:

The ``artificial`` attribute can be applied to an inline function. If such a 
function is inlined, the attribute indicates that debuggers should associate 
the resulting instructions with the call site, rather than with the 
corresponding line within the inlined callee.

+
+.. _GCC: https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Function-Attributes.html

If you still want to reference the GCC documentation, could you pick a newer 
version? :)

+  }];
+}

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=325081=325080=325081=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Feb 13 16:14:07 2018
@@ -3235,7 +3235,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
   if (Name.startswith("\01"))
 Name = Name.substr(1);

-  if (!HasDecl || D->isImplicit()) {
+  if (!HasDecl || D->isImplicit() || D->hasAttr()) {
 Flags |= llvm::DINode::FlagArtificial;
 // Artificial functions should not silently reuse CurLoc.
 CurLoc = SourceLocation();

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=325081=325080=325081=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (ori

Re: r325081 - Implement function attribute artificial

2018-02-13 Thread Richard Smith via cfe-commits 
On 13 February 2018 at 16:14, Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: erichkeane
> Date: Tue Feb 13 16:14:07 2018
> New Revision: 325081
>
> URL: http://llvm.org/viewvc/llvm-project?rev=325081=rev
> Log:
> Implement function attribute artificial
>
> Added support in clang for GCC function attribute 'artificial'. This
> attribute
> is used to control stepping behavior of debugger with respect to inline
> functions.
>
> Patch By: Elizabeth Andrews (eandrews)
>
> Differential Revision: https://reviews.llvm.org/D43259
>
>
> Added:
> cfe/trunk/test/CodeGen/artificial.c
> cfe/trunk/test/Sema/artificial.c
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/AttrDocs.td
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/Attr.td?rev=325081=325080=325081=diff
> 
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Tue Feb 13 16:14:07 2018
> @@ -111,6 +111,9 @@ def SharedVar : SubsetSubject  def GlobalVar : SubsetSubject   [{S->hasGlobalStorage()}], "global
> variables">;
>
> +def InlineFunction : SubsetSubject + [{S->isInlineSpecified()}], "inline
> functions">;
> +
>  // FIXME: this hack is needed because DeclNodes.td defines the base Decl
> node
>  // type to be a class, not a definition. This makes it impossible to
> create an
>  // attribute subject which accepts a Decl. Normally, this is not a
> problem,
> @@ -588,6 +591,12 @@ def AlwaysInline : InheritableAttr {
>let Documentation = [Undocumented];
>  }
>
> +def Artificial : InheritableAttr {
> +  let Spellings = [GCC<"artificial">];
> +  let Subjects = SubjectList<[InlineFunction], WarnDiag>;
> +  let Documentation = [ArtificialDocs];
> +}
> +
>  def XRayInstrument : InheritableAttr {
>let Spellings = [Clang<"xray_always_instrument">,
> Clang<"xray_never_instrument">];
>
> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Basic/AttrDocs.td?rev=325081=325080=325081=diff
> 
> ==
> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Tue Feb 13 16:14:07 2018
> @@ -3273,3 +3273,13 @@ For more information see
>  or `msvc documentation  pl-pl/cpp/cpp/selectany>`_.
>  }];
>  }
> +
> +def ArtificialDocs : Documentation {
> +  let Category = DocCatFunction;
> +  let Content = [{
> +The ``artificial`` attribute is used with inline functions to treat the
> inline
> +function as a unit while debugging. For more information see GCC_
> documentation.
>

I find this to be hard to understand. What does "treat the inline function
as a unit" actually mean? How about something like:

The ``artificial`` attribute can be applied to an inline function. If such
a function is inlined, the attribute indicates that debuggers should
associate the resulting instructions with the call site, rather than with
the corresponding line within the inlined callee.

+
> +.. _GCC: https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/
> Function-Attributes.html


If you still want to reference the GCC documentation, could you pick a
newer version? :)

+  }];
> +}
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
> CGDebugInfo.cpp?rev=325081=325080=325081=diff
> 
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Feb 13 16:14:07 2018
> @@ -3235,7 +3235,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
>if (Name.startswith("\01"))
>  Name = Name.substr(1);
>
> -  if (!HasDecl || D->isImplicit()) {
> +  if (!HasDecl || D->isImplicit() || D->hasAttr()) {
>  Flags |= llvm::DINode::FlagArtificial;
>  // Artificial functions should not silently reuse CurLoc.
>  CurLoc = SourceLocation();
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDeclAttr.cpp?rev=325081=325080=325081=diff
> 
> ==
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Feb 13 16:14:07 2018
> @@ -6057,6 +6057,9 @@ static void ProcessDeclAttribute(Sema 
>case AttributeList::AT_AlwaysInline:
>  handleAlwaysInlineAttr(S, D, Attr);
>  break;
> +  case AttributeList::AT_Artificial:
> +handleSimpleAttribute(S, D, Attr);
> +break;