Re: Getting fully qualified names of random qualtypes.

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
On Tue, Oct 13, 2015 at 1:33 PM, David Blaikie <[hidden email]> wrote:


Does it makes sense to upstream this functionality from cling into clang? 

Presumably clang needs to do this in places already (diagnostic messages, etc?)? Have you found any instances of that? Is there existing code doing that work that could be refactored into something reusable (that way it's being actually executed and tested within Clang's many codepaths, which is helpful to avoid bitrot, etc)

To the best of my knowledge, it doesn't exist. Certainly not in diagnostics, which try to reproduce what the user wrote, rather than what they would need to write in general. (In particular, things like template_A<nested_type_0, template_B<nested_type_1>> seem to never have sub-types get fully expanded. I've looked for this in clang, and it is possible I missed something, so happy for any pointers.

Clang is oriented to reproducing the source code as written, not what one would need to write to access it from a random namespace.

This is one of several bits of functionality where the comment in my code is: "one would think this is implemented somewhere in clang, but one would be wrong". (Generic fully-qualified lookup being another. Also implemented in cling, but is not very complicated, so not as big of a deal as this.)

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); for example the naming routine that can be tuned to give a fully qualified name often do not reliably handle the
nested template parameters (similar to what Sterling also found out).

Cheers,
Philippe.

PS. As an aside the code we have in cling/lib/Utils/AST.cpp does a bit more as it can drop some
of the template parameters when they are defaulted and keep some typedef (for example std::string).
The goal is to produce a 'normalized name' that is platform independent and suitable to use
as a persistent representation of the class;   This is used as part of ROOT I/O which enables storing
arbitrary C++ objects in files that can be read on multiple platforms and can be read even when
the class' schema changes.   This used to store many petabytes of experimental data that needs to
be read very quickly and need to also be readable many years later.

On 10/13/15 4:01 PM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 1:33 PM, David Blaikie <[hidden email]> wrote:


Does it makes sense to upstream this functionality from cling into clang? 

Presumably clang needs to do this in places already (diagnostic messages, etc?)? Have you found any instances of that? Is there existing code doing that work that could be refactored into something reusable (that way it's being actually executed and tested within Clang's many codepaths, which is helpful to avoid bitrot, etc)

To the best of my knowledge, it doesn't exist. Certainly not in diagnostics, which try to reproduce what the user wrote, rather than what they would need to write in general. (In particular, things like template_A<nested_type_0, template_B<nested_type_1>> seem to never have sub-types get fully expanded. I've looked for this in clang, and it is possible I missed something, so happy for any pointers.

Clang is oriented to reproducing the source code as written, not what one would need to write to access it from a random namespace.

This is one of several bits of functionality where the comment in my code is: "one would think this is implemented somewhere in clang, but one would be wrong". (Generic fully-qualified lookup being another. Also implemented in cling, but is not very complicated, so not as big of a deal as this.)


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
Hi Sterling,

> Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

Of course :).  I agree that this feature belongs in clang proper.

AST.cpp is pretty much standalone (depends only on clang and llvm)

Thanks,
Philippe.

PS.  A part that has not yet been integrated in AST.cpp is the adding of default
template parameters (for user classes where that parameter might matter for I/O)
and can be found in:
    https://root.cern.ch/gitweb?p=root.git;a=blob;f=core/metautils/src/TMetaUtils.cxx
in the routines
    ROOT::TMetaUtils::GetNormalizedType
    ROOT::TMetaUtils::AddDefaultParameters
    ROOT::TMetaUtils::KeepNParams

On 10/14/15 11:53 AM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
So clang folks, where should this new code to calculate the complete, fully qualified name of arbitrary qualtypes go? 

My best guess is as a new file in clang/lib/AST, but it's not a great fit.

Does the license as listed in the file (reproduced below) create any issues?

// This file is dual-licensed: you can choose to license it under the University
// of Illinois Open Source License or the GNU Lesser General Public License. See
// LICENSE.TXT for details.

On Thu, Oct 15, 2015 at 8:47 AM, Philippe Canal <[hidden email]> wrote:
Hi Sterling,

> Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

Of course :).  I agree that this feature belongs in clang proper.

AST.cpp is pretty much standalone (depends only on clang and llvm)

Thanks,
Philippe.

PS.  A part that has not yet been integrated in AST.cpp is the adding of default
template parameters (for user classes where that parameter might matter for I/O)
and can be found in:
    https://root.cern.ch/gitweb?p=root.git;a=blob;f=core/metautils/src/TMetaUtils.cxx
in the routines
    ROOT::TMetaUtils::GetNormalizedType
    ROOT::TMetaUtils::AddDefaultParameters
    ROOT::TMetaUtils::KeepNParams


On 10/14/15 11:53 AM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email][hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev

I think it should probably be part of ASTContext. I have no idea about the license. Richard?


On Wed, Oct 21, 2015, 2:27 AM Sterling Augustine via cfe-dev <[hidden email]> wrote:
So clang folks, where should this new code to calculate the complete, fully qualified name of arbitrary qualtypes go? 

My best guess is as a new file in clang/lib/AST, but it's not a great fit.

Does the license as listed in the file (reproduced below) create any issues?

// This file is dual-licensed: you can choose to license it under the University
// of Illinois Open Source License or the GNU Lesser General Public License. See
// LICENSE.TXT for details.

On Thu, Oct 15, 2015 at 8:47 AM, Philippe Canal <[hidden email]> wrote:
Hi Sterling,

> Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

Of course :).  I agree that this feature belongs in clang proper.

AST.cpp is pretty much standalone (depends only on clang and llvm)

Thanks,
Philippe.

PS.  A part that has not yet been integrated in AST.cpp is the adding of default
template parameters (for user classes where that parameter might matter for I/O)
and can be found in:
    https://root.cern.ch/gitweb?p=root.git;a=blob;f=core/metautils/src/TMetaUtils.cxx
in the routines
    ROOT::TMetaUtils::GetNormalizedType
    ROOT::TMetaUtils::AddDefaultParameters
    ROOT::TMetaUtils::KeepNParams


On 10/14/15 11:53 AM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email][hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
ASTContext isn't appropriate for utility code for tooling support; that should go in libTooling instead. The question is whether any of this functionality is useful for the compiler itself or just for tools that use it as a library, and I suspect it's only really useful for the latter.

Danny, can you comment on the licensing question?

On Tue, Oct 20, 2015 at 9:13 PM, Manuel Klimek via cfe-dev <[hidden email]> wrote:

I think it should probably be part of ASTContext. I have no idea about the license. Richard?


On Wed, Oct 21, 2015, 2:27 AM Sterling Augustine via cfe-dev <[hidden email]> wrote:
So clang folks, where should this new code to calculate the complete, fully qualified name of arbitrary qualtypes go? 

My best guess is as a new file in clang/lib/AST, but it's not a great fit.

Does the license as listed in the file (reproduced below) create any issues?

// This file is dual-licensed: you can choose to license it under the University
// of Illinois Open Source License or the GNU Lesser General Public License. See
// LICENSE.TXT for details.

On Thu, Oct 15, 2015 at 8:47 AM, Philippe Canal <[hidden email]> wrote:
Hi Sterling,

> Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

Of course :).  I agree that this feature belongs in clang proper.

AST.cpp is pretty much standalone (depends only on clang and llvm)

Thanks,
Philippe.

PS.  A part that has not yet been integrated in AST.cpp is the adding of default
template parameters (for user classes where that parameter might matter for I/O)
and can be found in:
    https://root.cern.ch/gitweb?p=root.git;a=blob;f=core/metautils/src/TMetaUtils.cxx
in the routines
    ROOT::TMetaUtils::GetNormalizedType
    ROOT::TMetaUtils::AddDefaultParameters
    ROOT::TMetaUtils::KeepNParams


On 10/14/15 11:53 AM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email][hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
On Wed, Oct 21, 2015 at 8:08 AM Richard Smith <[hidden email]> wrote:
ASTContext isn't appropriate for utility code for tooling support; that should go in libTooling instead. The question is whether any of this functionality is useful for the compiler itself or just for tools that use it as a library, and I suspect it's only really useful for the latter.

Good point, in that case I'd put it somewhere in Tooling/Core. cc'ing Benjamin, who had (planned) a patch to open up a new source file there anyway.
 

Danny, can you comment on the licensing question?

On Tue, Oct 20, 2015 at 9:13 PM, Manuel Klimek via cfe-dev <[hidden email]> wrote:

I think it should probably be part of ASTContext. I have no idea about the license. Richard?


On Wed, Oct 21, 2015, 2:27 AM Sterling Augustine via cfe-dev <[hidden email]> wrote:
So clang folks, where should this new code to calculate the complete, fully qualified name of arbitrary qualtypes go? 

My best guess is as a new file in clang/lib/AST, but it's not a great fit.

Does the license as listed in the file (reproduced below) create any issues?

// This file is dual-licensed: you can choose to license it under the University
// of Illinois Open Source License or the GNU Lesser General Public License. See
// LICENSE.TXT for details.

On Thu, Oct 15, 2015 at 8:47 AM, Philippe Canal <[hidden email]> wrote:
Hi Sterling,

> Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

Of course :).  I agree that this feature belongs in clang proper.

AST.cpp is pretty much standalone (depends only on clang and llvm)

Thanks,
Philippe.

PS.  A part that has not yet been integrated in AST.cpp is the adding of default
template parameters (for user classes where that parameter might matter for I/O)
and can be found in:
    https://root.cern.ch/gitweb?p=root.git;a=blob;f=core/metautils/src/TMetaUtils.cxx
in the routines
    ROOT::TMetaUtils::GetNormalizedType
    ROOT::TMetaUtils::AddDefaultParameters
    ROOT::TMetaUtils::KeepNParams


On 10/14/15 11:53 AM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email][hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
The license itself is fine.


On Wed, Oct 21, 2015 at 1:04 AM, Manuel Klimek <[hidden email]> wrote:
On Wed, Oct 21, 2015 at 8:08 AM Richard Smith <[hidden email]> wrote:
ASTContext isn't appropriate for utility code for tooling support; that should go in libTooling instead. The question is whether any of this functionality is useful for the compiler itself or just for tools that use it as a library, and I suspect it's only really useful for the latter.

Good point, in that case I'd put it somewhere in Tooling/Core. cc'ing Benjamin, who had (planned) a patch to open up a new source file there anyway.
 

Danny, can you comment on the licensing question?

On Tue, Oct 20, 2015 at 9:13 PM, Manuel Klimek via cfe-dev <[hidden email]> wrote:

I think it should probably be part of ASTContext. I have no idea about the license. Richard?


On Wed, Oct 21, 2015, 2:27 AM Sterling Augustine via cfe-dev <[hidden email]> wrote:
So clang folks, where should this new code to calculate the complete, fully qualified name of arbitrary qualtypes go? 

My best guess is as a new file in clang/lib/AST, but it's not a great fit.

Does the license as listed in the file (reproduced below) create any issues?

// This file is dual-licensed: you can choose to license it under the University
// of Illinois Open Source License or the GNU Lesser General Public License. See
// LICENSE.TXT for details.

On Thu, Oct 15, 2015 at 8:47 AM, Philippe Canal <[hidden email]> wrote:
Hi Sterling,

> Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

Of course :).  I agree that this feature belongs in clang proper.

AST.cpp is pretty much standalone (depends only on clang and llvm)

Thanks,
Philippe.

PS.  A part that has not yet been integrated in AST.cpp is the adding of default
template parameters (for user classes where that parameter might matter for I/O)
and can be found in:
    https://root.cern.ch/gitweb?p=root.git;a=blob;f=core/metautils/src/TMetaUtils.cxx
in the routines
    ROOT::TMetaUtils::GetNormalizedType
    ROOT::TMetaUtils::AddDefaultParameters
    ROOT::TMetaUtils::KeepNParams


On 10/14/15 11:53 AM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email][hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev




_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
Specifially, Benjamin has a patch out that adds a Tooling/Core/Lookup.h where this would fit in quite nicely.

On Wed, Oct 21, 2015 at 5:31 PM Daniel Berlin <[hidden email]> wrote:
The license itself is fine.


On Wed, Oct 21, 2015 at 1:04 AM, Manuel Klimek <[hidden email]> wrote:
On Wed, Oct 21, 2015 at 8:08 AM Richard Smith <[hidden email]> wrote:
ASTContext isn't appropriate for utility code for tooling support; that should go in libTooling instead. The question is whether any of this functionality is useful for the compiler itself or just for tools that use it as a library, and I suspect it's only really useful for the latter.

Good point, in that case I'd put it somewhere in Tooling/Core. cc'ing Benjamin, who had (planned) a patch to open up a new source file there anyway.
 

Danny, can you comment on the licensing question?

On Tue, Oct 20, 2015 at 9:13 PM, Manuel Klimek via cfe-dev <[hidden email]> wrote:

I think it should probably be part of ASTContext. I have no idea about the license. Richard?


On Wed, Oct 21, 2015, 2:27 AM Sterling Augustine via cfe-dev <[hidden email]> wrote:
So clang folks, where should this new code to calculate the complete, fully qualified name of arbitrary qualtypes go? 

My best guess is as a new file in clang/lib/AST, but it's not a great fit.

Does the license as listed in the file (reproduced below) create any issues?

// This file is dual-licensed: you can choose to license it under the University
// of Illinois Open Source License or the GNU Lesser General Public License. See
// LICENSE.TXT for details.

On Thu, Oct 15, 2015 at 8:47 AM, Philippe Canal <[hidden email]> wrote:
Hi Sterling,

> Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

Of course :).  I agree that this feature belongs in clang proper.

AST.cpp is pretty much standalone (depends only on clang and llvm)

Thanks,
Philippe.

PS.  A part that has not yet been integrated in AST.cpp is the adding of default
template parameters (for user classes where that parameter might matter for I/O)
and can be found in:
    https://root.cern.ch/gitweb?p=root.git;a=blob;f=core/metautils/src/TMetaUtils.cxx
in the routines
    ROOT::TMetaUtils::GetNormalizedType
    ROOT::TMetaUtils::AddDefaultParameters
    ROOT::TMetaUtils::KeepNParams


On 10/14/15 11:53 AM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email][hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev




_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
I copied these file directly from Cling to Tooling/Core/, and renamed them QualTypeNames.[cpp|h].

It compiles and works just fine as a direct copy.

I have made a couple of very minor changes, such as moving the enclosing namespace from cling to clang. Shall I do a more substantial clang-style edit?



On Thu, Oct 22, 2015 at 5:29 AM, Manuel Klimek <[hidden email]> wrote:
Specifially, Benjamin has a patch out that adds a Tooling/Core/Lookup.h where this would fit in quite nicely.

On Wed, Oct 21, 2015 at 5:31 PM Daniel Berlin <[hidden email]> wrote:
The license itself is fine.


On Wed, Oct 21, 2015 at 1:04 AM, Manuel Klimek <[hidden email]> wrote:
On Wed, Oct 21, 2015 at 8:08 AM Richard Smith <[hidden email]> wrote:
ASTContext isn't appropriate for utility code for tooling support; that should go in libTooling instead. The question is whether any of this functionality is useful for the compiler itself or just for tools that use it as a library, and I suspect it's only really useful for the latter.

Good point, in that case I'd put it somewhere in Tooling/Core. cc'ing Benjamin, who had (planned) a patch to open up a new source file there anyway.
 

Danny, can you comment on the licensing question?

On Tue, Oct 20, 2015 at 9:13 PM, Manuel Klimek via cfe-dev <[hidden email]> wrote:

I think it should probably be part of ASTContext. I have no idea about the license. Richard?


On Wed, Oct 21, 2015, 2:27 AM Sterling Augustine via cfe-dev <[hidden email]> wrote:
So clang folks, where should this new code to calculate the complete, fully qualified name of arbitrary qualtypes go? 

My best guess is as a new file in clang/lib/AST, but it's not a great fit.

Does the license as listed in the file (reproduced below) create any issues?

// This file is dual-licensed: you can choose to license it under the University
// of Illinois Open Source License or the GNU Lesser General Public License. See
// LICENSE.TXT for details.

On Thu, Oct 15, 2015 at 8:47 AM, Philippe Canal <[hidden email]> wrote:
Hi Sterling,

> Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

Of course :).  I agree that this feature belongs in clang proper.

AST.cpp is pretty much standalone (depends only on clang and llvm)

Thanks,
Philippe.

PS.  A part that has not yet been integrated in AST.cpp is the adding of default
template parameters (for user classes where that parameter might matter for I/O)
and can be found in:
    https://root.cern.ch/gitweb?p=root.git;a=blob;f=core/metautils/src/TMetaUtils.cxx
in the routines
    ROOT::TMetaUtils::GetNormalizedType
    ROOT::TMetaUtils::AddDefaultParameters
    ROOT::TMetaUtils::KeepNParams


On 10/14/15 11:53 AM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email][hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev





_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev

The right next step is to send a patch / code review where we can discuss the change. Is there one that I have missed?


On Thu, Oct 22, 2015, 10:57 PM Sterling Augustine <[hidden email]> wrote:
I copied these file directly from Cling to Tooling/Core/, and renamed them QualTypeNames.[cpp|h].

It compiles and works just fine as a direct copy.

I have made a couple of very minor changes, such as moving the enclosing namespace from cling to clang. Shall I do a more substantial clang-style edit?



On Thu, Oct 22, 2015 at 5:29 AM, Manuel Klimek <[hidden email]> wrote:
Specifially, Benjamin has a patch out that adds a Tooling/Core/Lookup.h where this would fit in quite nicely.

On Wed, Oct 21, 2015 at 5:31 PM Daniel Berlin <[hidden email]> wrote:
The license itself is fine.


On Wed, Oct 21, 2015 at 1:04 AM, Manuel Klimek <[hidden email]> wrote:
On Wed, Oct 21, 2015 at 8:08 AM Richard Smith <[hidden email]> wrote:
ASTContext isn't appropriate for utility code for tooling support; that should go in libTooling instead. The question is whether any of this functionality is useful for the compiler itself or just for tools that use it as a library, and I suspect it's only really useful for the latter.

Good point, in that case I'd put it somewhere in Tooling/Core. cc'ing Benjamin, who had (planned) a patch to open up a new source file there anyway.
 

Danny, can you comment on the licensing question?

On Tue, Oct 20, 2015 at 9:13 PM, Manuel Klimek via cfe-dev <[hidden email]> wrote:

I think it should probably be part of ASTContext. I have no idea about the license. Richard?


On Wed, Oct 21, 2015, 2:27 AM Sterling Augustine via cfe-dev <[hidden email]> wrote:
So clang folks, where should this new code to calculate the complete, fully qualified name of arbitrary qualtypes go? 

My best guess is as a new file in clang/lib/AST, but it's not a great fit.

Does the license as listed in the file (reproduced below) create any issues?

// This file is dual-licensed: you can choose to license it under the University
// of Illinois Open Source License or the GNU Lesser General Public License. See
// LICENSE.TXT for details.

On Thu, Oct 15, 2015 at 8:47 AM, Philippe Canal <[hidden email]> wrote:
Hi Sterling,

> Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

Of course :).  I agree that this feature belongs in clang proper.

AST.cpp is pretty much standalone (depends only on clang and llvm)

Thanks,
Philippe.

PS.  A part that has not yet been integrated in AST.cpp is the adding of default
template parameters (for user classes where that parameter might matter for I/O)
and can be found in:
    https://root.cern.ch/gitweb?p=root.git;a=blob;f=core/metautils/src/TMetaUtils.cxx
in the routines
    ROOT::TMetaUtils::GetNormalizedType
    ROOT::TMetaUtils::AddDefaultParameters
    ROOT::TMetaUtils::KeepNParams


On 10/14/15 11:53 AM, Sterling Augustine wrote:
On Tue, Oct 13, 2015 at 2:48 PM, Philippe Canal <[hidden email][hidden email]> wrote:
Hi,

We also discovered that we could not find in clang a routine that reliably generate the name (or the equivalent
decl/type) corresponding to a type as it would need to be typed from the global namespace (i.e. a unique identifier
of the class); 

Philippe,

Would you all be amenable to contributing cling/lib/Utils/AST.cpp to clang?

I would be willing to write-up the patch if that would be helpful. I'm not sure how deep the dependencies go.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev





_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
On Fri, Oct 23, 2015 at 2:07 AM, Manuel Klimek <[hidden email]> wrote:

The right next step is to send a patch / code review where we can discuss the change. Is there one that I have missed?

This required a bit of  editing, so I've now trimmed it to just the portions that I need. Which makes the patch quite a bit smaller. So there's that.

Enclosed. It is likely I missed some stylistic issues, but clang-format -llvm has done some magic too.

Let me know the next steps.

Also, I want to be sure the Cling project is OK with the way it is attributed. Happy to work with you all however you like there.



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

qualtype_names.patch (73K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
I believe this patch will take a couple more steps to 'clangify' it.
Can you submit it via the normal patch review process to cfe-commits (optimally via phabricator, so the code review is easier)

Thanks!

On Fri, Oct 23, 2015 at 9:06 PM Sterling Augustine <[hidden email]> wrote:
On Fri, Oct 23, 2015 at 2:07 AM, Manuel Klimek <[hidden email]> wrote:

The right next step is to send a patch / code review where we can discuss the change. Is there one that I have missed?

This required a bit of  editing, so I've now trimmed it to just the portions that I need. Which makes the patch quite a bit smaller. So there's that.

Enclosed. It is likely I missed some stylistic issues, but clang-format -llvm has done some magic too.

Let me know the next steps.

Also, I want to be sure the Cling project is OK with the way it is attributed. Happy to work with you all however you like there.



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
In reply to this post by James Y Knight via cfe-dev
On 23/10/15 21:06, Sterling Augustine wrote:
> Also, I want to be sure the Cling project is OK with the way it is
> attributed. Happy to work with you all however you like there.
>
I am more than happy to attribute this part of the code. I'd like as
many parts of cling as possible to go upstream.

Could you add Phillipe as a co-author? Eg.

+// author:  Vassil Vassilev<[hidden email]>
+// author:  Philippe Canal <[hidden email]>
 

Out of curiosity: what do you need this for?

Vassil
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
On Sat, Oct 24, 2015 at 10:40 AM, Vassil Vassilev <[hidden email]> wrote:

> On 23/10/15 21:06, Sterling Augustine wrote:
>>
>> Also, I want to be sure the Cling project is OK with the way it is
>> attributed. Happy to work with you all however you like there.
>>
> I am more than happy to attribute this part of the code. I'd like as many
> parts of cling as possible to go upstream.
>
> Could you add Phillipe as a co-author? Eg.
>
> +// author:  Vassil Vassilev<[hidden email]>
> +// author:  Philippe Canal <[hidden email]>
>
> Out of curiosity: what do you need this for?

I'm working on a tool that examines a header file and describes
exactly how one would call the functions it declares--detects the
namespaces and classes they are in. As part of that, it needs to
determine how to declare the types of the parameters they take.

Apparently Clang doesn't allow file-level authorship comments, so I'll
remove that detail if it is OK with you. If it isn't, I'll just
incorporate cling/AST.[cpp|h]  directly into my own project--that will
be quicker than the reformatting and refactoring rabbit hole anyway.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
Hi Sterling,

 > Apparently Clang doesn't allow file-level authorship comments, so I'll
 > remove that detail if it is OK with you.

It is okay with us.  Could you add an authorship/provenance information in the commit log
or a credit file if there is one.

Thanks
Philippe.

On 10/26/15 11:34 AM, Sterling Augustine wrote:

> On Sat, Oct 24, 2015 at 10:40 AM, Vassil Vassilev <[hidden email]> wrote:
>> On 23/10/15 21:06, Sterling Augustine wrote:
>>> Also, I want to be sure the Cling project is OK with the way it is
>>> attributed. Happy to work with you all however you like there.
>>>
>> I am more than happy to attribute this part of the code. I'd like as many
>> parts of cling as possible to go upstream.
>>
>> Could you add Phillipe as a co-author? Eg.
>>
>> +// author:  Vassil Vassilev<[hidden email]>
>> +// author:  Philippe Canal <[hidden email]>
>>
>> Out of curiosity: what do you need this for?
> I'm working on a tool that examines a header file and describes
> exactly how one would call the functions it declares--detects the
> namespaces and classes they are in. As part of that, it needs to
> determine how to declare the types of the parameters they take.
>
> Apparently Clang doesn't allow file-level authorship comments, so I'll
> remove that detail if it is OK with you. If it isn't, I'll just
> incorporate cling/AST.[cpp|h]  directly into my own project--that will
> be quicker than the reformatting and refactoring rabbit hole anyway.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
In reply to this post by James Y Knight via cfe-dev


On Mon, Oct 26, 2015 at 5:34 PM Sterling Augustine <[hidden email]> wrote:
On Sat, Oct 24, 2015 at 10:40 AM, Vassil Vassilev <[hidden email]> wrote:
> On 23/10/15 21:06, Sterling Augustine wrote:
>>
>> Also, I want to be sure the Cling project is OK with the way it is
>> attributed. Happy to work with you all however you like there.
>>
> I am more than happy to attribute this part of the code. I'd like as many
> parts of cling as possible to go upstream.
>
> Could you add Phillipe as a co-author? Eg.
>
> +// author:  Vassil Vassilev<[hidden email]>
> +// author:  Philippe Canal <[hidden email]>
>
> Out of curiosity: what do you need this for?

I'm working on a tool that examines a header file and describes
exactly how one would call the functions it declares--detects the
namespaces and classes they are in. As part of that, it needs to
determine how to declare the types of the parameters they take.

Apparently Clang doesn't allow file-level authorship comments, so I'll
remove that detail if it is OK with you. If it isn't, I'll just
incorporate cling/AST.[cpp|h]  directly into my own project--that will
be quicker than the reformatting and refactoring rabbit hole anyway.

While it is most certainly some effort to get it right, the cool thing is that in the end we have something that works for multiple projects, thus magnifying the impact. Also, you'll get automatic maintenance, which is often quite some work down the road ...

Cheers,
/Manuel 

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Getting fully qualified names of random qualtypes.

James Y Knight via cfe-dev
In reply to this post by James Y Knight via cfe-dev
On 26/10/15 09:34, Sterling Augustine wrote:

> On Sat, Oct 24, 2015 at 10:40 AM, Vassil Vassilev <[hidden email]> wrote:
>> On 23/10/15 21:06, Sterling Augustine wrote:
>>> Also, I want to be sure the Cling project is OK with the way it is
>>> attributed. Happy to work with you all however you like there.
>>>
>> I am more than happy to attribute this part of the code. I'd like as many
>> parts of cling as possible to go upstream.
>>
>> Could you add Phillipe as a co-author? Eg.
>>
>> +// author:  Vassil Vassilev<[hidden email]>
>> +// author:  Philippe Canal <[hidden email]>
>>
>> Out of curiosity: what do you need this for?
> I'm working on a tool that examines a header file and describes
> exactly how one would call the functions it declares--detects the
> namespaces and classes they are in. As part of that, it needs to
> determine how to declare the types of the parameters they take.
Thanks for the info, sounds cool ;)
>
> Apparently Clang doesn't allow file-level authorship comments, so I'll
> remove that detail if it is OK with you. If it isn't, I'll just
> incorporate cling/AST.[cpp|h]  directly into my own project--that will
> be quicker than the reformatting and refactoring rabbit hole anyway.
The authorship comments could be transformed as Philippe suggested.

I think we should try to aim for an "official" patch review. IMO
investing some extra time in refactoring and reformatting to put the
work mainline will definitely pay off in long term (if you plan your
tool to be around for a long time). Maybe Philippe and I could help...
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev