Re: Need to use function "getAsCXXRecordDecl" of ASTMatchFinder.cpp in a clang-tidy check

2016-02-10 Thread Cong Liu via cfe-commits
Hi Richard,

You are right. Actually I just need to consider the cases that there are
full instantiations of the primary template. I have changed the strategy
and not use that function.

Thanks a lot!
Cong

On Wed, Feb 10, 2016 at 2:17 AM, Richard Smith 
wrote:

> On Tue, Feb 9, 2016 at 4:15 PM, Cong Liu  wrote:
> > Hi Richard,
> >
> > Thank you for your reply. Yes, the case I need to deal with is like what
> you
> > said:
> >>
> >> If you want to make the assumption that the primary template will be
> >> used for an unknown specialization, you'll need something like that
> >> function in ASTMatchFinder.
> >
> >
> > For example,
> >
> >   template 
> >   struct Base {};
> >   template 
> >   struct Derived : Base{};
> >
> >   Derived T1;
> >
> > In this case, I need to firstly get the CXXBaseSpecifier from line 4,
> then
> > get the QualType of primary template (Base), then get its declaration.
> > For this case, that function in ASTMatchFinder works but
> > Type::getAsCXXRecordDecl does not.
> >
> > So, what do you suggest?
>
> Using that may not be correct when analysing virtual function
> overrides. Consider this:
>
> template struct Base {
>   virtual void f(T);
> };
> template<> struct Base {
>   virtual void f();
> };
> template::value> struct Derived :
> Base {
>   virtual void f();
> };
> template struct Derived : Base {
>   virtual void f(T);
> };
>
> Here, it would be wrong to report that the Derived primary template
> fails to override a virtual function from the Base primary template,
> as the Derived primary template is actually only ever used when T ==
> void, and there's a specialization of the Base template for that case.
> I have no idea whether that's acceptable for your check.
>
> In principle, I'm fine with us moving the functionality in
> ASTMatchFinder (that I recently renamed to
> getAsCXXRecordDeclOrPrimaryTemplate to better express its purpose) to
> somewhere more central, but my concern is that most uses of it will in
> fact be subtle bugs.
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: Need to use function "getAsCXXRecordDecl" of ASTMatchFinder.cpp in a clang-tidy check

2016-02-09 Thread Richard Smith via cfe-commits
On Tue, Feb 9, 2016 at 9:03 AM, Cong Liu via cfe-commits
 wrote:
> Hi Richard,
>
> I need to use the function (line 747 of ASTMatchFinder.cpp):
>
>   static CXXRecordDecl *getAsCXXRecordDecl(const Type *TypeNode)
>
> in the misc-virtual-near-miss check of clang-tidy, because it can correctly
> get the declaration of a template type, while QualType::getAsCXXRecordDecl
> cannot.

In what sense correctly? Type::getAsCXXRecordDecl should get the
CXXRecordDecl in every case where we know which record will be used.
If you want to make the assumption that the primary template will be
used for an unknown specialization, you'll need something like that
function in ASTMatchFinder, but it's not generally correct to make
that assumption. I'm not really sure why we're using this
sometimes-incorrect mechanism in MatchASTVisitor:classIsDerivedFrom;
that seems like a bug.

> But this function is not accessible in clang-tidy check. I think we should
> make this function public accessible since its useful. Could you please tell
> me how to do it? Thanks a lot.
>
> Best regards,
> Cong
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Need to use function "getAsCXXRecordDecl" of ASTMatchFinder.cpp in a clang-tidy check

2016-02-09 Thread Cong Liu via cfe-commits
Hi Richard,

I need to use the function (line 747 of ASTMatchFinder.cpp
):

  static CXXRecordDecl *getAsCXXRecordDecl(const Type *TypeNode)

in the misc-virtual-near-miss check of clang-tidy, because it can correctly
get the declaration of a template type, while QualType::getAsCXXRecordDecl
cannot.

But this function is not accessible in clang-tidy check. I think we should
make this function public accessible since its useful. Could you please
tell me how to do it? Thanks a lot.

Best regards,
Cong
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: Need to use function "getAsCXXRecordDecl" of ASTMatchFinder.cpp in a clang-tidy check

2016-02-09 Thread Cong Liu via cfe-commits
Hi Richard,

Thank you for your reply. Yes, the case I need to deal with is like what
you said:

> If you want to make the assumption that the primary template will be
> used for an unknown specialization, you'll need something like that
> function in ASTMatchFinder.


For example,


   1.   template 
   2.   struct Base {};
   3.   template 
   4.   struct Derived : Base{};
   5.
   6.   Derived T1;

In this case, I need to firstly get the CXXBaseSpecifier from line 4, then
get the QualType of primary template (Base), then get its declaration.
For this case, that function in ASTMatchFinder works but
Type::getAsCXXRecordDecl does not.

So, what do you suggest?

Best regards,
Cong
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: Need to use function "getAsCXXRecordDecl" of ASTMatchFinder.cpp in a clang-tidy check

2016-02-09 Thread Richard Smith via cfe-commits
On Tue, Feb 9, 2016 at 4:15 PM, Cong Liu  wrote:
> Hi Richard,
>
> Thank you for your reply. Yes, the case I need to deal with is like what you
> said:
>>
>> If you want to make the assumption that the primary template will be
>> used for an unknown specialization, you'll need something like that
>> function in ASTMatchFinder.
>
>
> For example,
>
>   template 
>   struct Base {};
>   template 
>   struct Derived : Base{};
>
>   Derived T1;
>
> In this case, I need to firstly get the CXXBaseSpecifier from line 4, then
> get the QualType of primary template (Base), then get its declaration.
> For this case, that function in ASTMatchFinder works but
> Type::getAsCXXRecordDecl does not.
>
> So, what do you suggest?

Using that may not be correct when analysing virtual function
overrides. Consider this:

template struct Base {
  virtual void f(T);
};
template<> struct Base {
  virtual void f();
};
template::value> struct Derived : Base {
  virtual void f();
};
template struct Derived : Base {
  virtual void f(T);
};

Here, it would be wrong to report that the Derived primary template
fails to override a virtual function from the Base primary template,
as the Derived primary template is actually only ever used when T ==
void, and there's a specialization of the Base template for that case.
I have no idea whether that's acceptable for your check.

In principle, I'm fine with us moving the functionality in
ASTMatchFinder (that I recently renamed to
getAsCXXRecordDeclOrPrimaryTemplate to better express its purpose) to
somewhere more central, but my concern is that most uses of it will in
fact be subtle bugs.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits