Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

template parameter std::enable_if_t not parsed #2305

Closed
slnj opened this issue Feb 5, 2022 · 18 comments · Fixed by #2306 or #2316
Closed

template parameter std::enable_if_t not parsed #2305

slnj opened this issue Feb 5, 2022 · 18 comments · Fixed by #2306 or #2316
Assignees
Labels
Milestone

Comments

@slnj
Copy link

slnj commented Feb 5, 2022

Hi,
I have some parsing issues using version 2.0.6.
It seems the problem occurs when I use std::enable_if_t macro helper in templates

There is not problem using std::enable_if like:

template < typename T , typename std::enable_if<std::is_base_of<BaseT, T>::value, int>::type = 0 >
class A
{
....
};

But parsing fails if using similar but simplified syntax using std::enable_if_t :

template < typename T ,  std::enable_if_t<std::is_base_of<BaseT, T>::value, int> = 0 > 
class A
{
....
};

Yo can see it usage at the "Notes" section at https://en.cppreference.com/w/cpp/types/enable_if

Both syntax have similar functionality, only that enable_if_t adds the "template" and ":type" on it definition (can see a type_traits header file), so both "template" and ":type" are not needed when used.
std::enable_if_t helper is specified since C++14 ( according to cppreference )

It's expected both alternatives to be recognized, but only the fist one is.

@guwirth
Copy link
Collaborator

guwirth commented Feb 5, 2022

Hi @slnj,

thx for your feedback, we will have a look.

Could you provide also the error message you get please?
What is the impact of this error (something else not working)?

Regards,

@slnj
Copy link
Author

slnj commented Feb 5, 2022

Hi Günter,
I don't know if I'm understanding this correctly.
I have a lot of messages like on the template lines:
C++ Parser can't read code. Declaration is skipped (last token='>', line=32, column=95)

But I have sonar.cxx.errorRecoveryEnabled = true.
If I change this option to false, all the errors go away.

Maybe I'm not understanding things correctly and these errors are expected when errorRecoveryEnabled=true

@slnj
Copy link
Author

slnj commented Feb 5, 2022

Correction: Even with errorRecoveryEnabled=false, get error but changed to (this is a sample):

Parse error at line 27 column 98: 26: & amount); --> template<typename T, std::enable_if_t< tens::is_numeric_v<T> && std::is_integral_v<T>, int> = 0 > 28: SCBigNumber( T num ): decimalPlaces_{decimalPlaces_DEFAULT}, value_{num} {} 29: 30: SCBigNumber(int decimalPlaces, const std::string& amount 

And only appears one error in each file, only on the fist template declaration

@guwirth
Copy link
Collaborator

guwirth commented Feb 6, 2022

Hi @slnj,

sonar.cxx.errorRecoveryEnabled

The difference is explained here:

sonar.cxx.errorRecoveryEnabled = true is the default and should be used because sonar.cxx.errorRecoveryEnabled = false breaks after first error.

The problem in your case seems to be the type traits. The cxx parser implements a C++ grammer and type traits are not part of it. ::type and ::value are only a naming convention within templates. We added some syntax sugar to support it without reading the header files. But it seems that your case is not supported yet.

But actually it doesn't matter, because the "correct syntax" is only needed for some cxx rules (https://github.com/SonarOpenCommunity/sonar-cxx/wiki/CXX-Rules) and the correct calculation of the software metrics. Reports from external tools can also be assigned correctly despite syntax errors.

Until the error is fixed, you should normally be able to ignore the error message.

Regards,

@slnj
Copy link
Author

slnj commented Feb 9, 2022

Thanks Günter,
I'll keep sonar.cxx.errorRecoveryEnabled=true as recommended and ignore this errors

Regards,

@slnj slnj closed this as completed Feb 9, 2022
@guwirth
Copy link
Collaborator

guwirth commented Feb 9, 2022

Keep it open for further investigations.

@guwirth guwirth reopened this Feb 9, 2022
@guwirth
Copy link
Collaborator

guwirth commented Feb 9, 2022

template < typename T ,          std::enable_if_t<std::is_base_of<BaseT, T>::value, int>       = 0 > class A {}; // error
template < typename T , typename std::enable_if  <std::is_base_of<BaseT, T>::value, int>::type = 0 > class A {}; // ok

b.rule(typeTraits).is( // syntax sugar to handle type traits ...::type (not part of C++ grammar)

@guwirth guwirth added bug and removed question labels Feb 9, 2022
@guwirth
Copy link
Collaborator

guwirth commented Feb 10, 2022

Issue is in templateParameter detection (see below). Parser does not detect that std::enable_if_t returns a type name and a default value = 0 can be assigned:

template < typename T ,  std::enable_if_t<std::is_base_of<BaseT, T>::value, int> = 0 > class A {}; // error
template < typename T ,  std::enable_if_t<std::is_base_of<BaseT, T>::value, int>     > class A {}; // ok

grafik

guwirth added a commit to guwirth/sonar-cxx that referenced this issue Feb 11, 2022
- supporting type traits helper as typeParameter (e.g. std::enable_if_t)
- close SonarOpenCommunity#2305

```C++
template <typename T, typename std::enable_if  <std::is_base_of<BaseT, T>::value, int>::type = 0> class A {};
template <typename T,          std::enable_if_t<std::is_base_of<BaseT, T>::value, int>       = 0> class C {};
```
@guwirth guwirth added this to the 2.0.7 milestone Feb 11, 2022
@guwirth guwirth self-assigned this Feb 11, 2022
@guwirth
Copy link
Collaborator

guwirth commented Feb 11, 2022

Hi @slnj,

there is a fix available: #2306

You can download and try with this version:
https://ci.appveyor.com/project/SonarOpenCommunity/sonar-cxx/branch/master/artifacts

Please give us feedback.

Regards,

@slnj
Copy link
Author

slnj commented Feb 13, 2022

Hi Günter,
I'm checking with sslr toolkit from your link. ( First time, I've never used it, and it's great! )
I tried with this:

class A 
{
public:
    template<typename T, std::enable_if_t< std::is_integral_t<T>, int> = 0 >
    A( T num ) {}
    
    template<typename T, std::enable_if_t< std::is_integral_v<T>, int> = 0 >
    int foo( T num ) { }
};

I get an error on the constructor, but if instead I use template<typename T> it is accepted.
Even with template<typename T, std::enable_if_t< std::is_integral_v<T>, int> > it is accepted.
On the member there is no error also.

It seems the enable_if<.....> = 0 is not accepted on contructors but it is accepted on member functions

@slnj
Copy link
Author

slnj commented Feb 13, 2022

The same happens with the operators:
In the previous sample:

 template< typename T, std::enable_if_t< std::is_integral_v<T>, int> > 
    A operator+( T num ) {  }

is accepted but

 template< typename T, std::enable_if_t< std::is_integral_v<T>, int>  = 0> 
    A operator+( T num ) {  }

is not

@guwirth guwirth reopened this Feb 14, 2022
@guwirth
Copy link
Collaborator

guwirth commented Feb 14, 2022

Hello @slnj,

is a bigger thing.

All places where an implicit conversion to typename can take place must be checked. I'm not sure if we can do this adapting the grammar. The cxx plugin is verifying only the grammar (syntax check) without a semantic check.

Regards,

@slnj
Copy link
Author

slnj commented Feb 14, 2022

But this is not about enable_if_t.
Also enable_if is not parsed for constructor/operators (but it is for member functions):

    template<typename T, typename std::enable_if< std::is_integral<T>::value, int>::type = 0 >
    A( T num ) {}

@guwirth
Copy link
Collaborator

guwirth commented Feb 17, 2022

Hi @slnj,

The reduction makes the problem more transparent. It is a ambiguous grammar problem with = 0 > A. In case there is a keyword after > (e.g. struct, class) it works fine.

class A {
   template<typename T = 0 > A ( T num ) {}
};

Not sure if we can solve this because parser has no semantic knowledge. Maybe we can add some syntax sugar for most common cases?

  • = 0
  • ??? (@slnj What are typical values in such constructs?)

Regards,

@guwirth
Copy link
Collaborator

guwirth commented Feb 18, 2022

cases I found in VC and Boost:

  • ... = 0
  • ... = nullptr
  • ... = true
template <typename Collection, std::enable_if_t<std::is_convertible_v<Collection, interface_type>>* = nullptr>
template <class T> void _Cleanup(T* /*values*/, unsigned /*actual*/, typename enable_if<!__is_class(T) && !(is_pointer<T>::value)>::type = 0) {}
template <typename T, std::enable_if_t<is_implements_v<T>, int> = 0>
template<class T> void destroy(T*, typename std::enable_if<std::is_trivially_destructible<T>::value>::type* = 0) {}
template<class T, typename std::enable_if<!std::is_trivially_destructible<T>{} && (std::is_class<T>{} || std::is_union<T>{}), bool>::type = true>

@guwirth
Copy link
Collaborator

guwirth commented Feb 18, 2022

Hi @slnj,

please try again, there is a new fix available.

You can download and try with this version:
https://ci.appveyor.com/project/SonarOpenCommunity/sonar-cxx/branch/master/artifacts

Regards,

@guwirth guwirth closed this as completed Feb 19, 2022
@slnj
Copy link
Author

slnj commented Feb 20, 2022

Hi Günter,

I tried with sslr toolkit and it seems this new version solves the problem!!

And yes, I use ..., int>::type = 0 > but also ..., bool>::type = true > and ...>::type* = nullptr >
( If used enable_if_t instead of enable_if, the last ::type keyword should be removed)
It's just to assign a valid value for the returned type.
If enable_if condition is true, type will be returned and assignment is valid, and when the condition is not true, no type will be returned and the assignment will fail making the template not to be considered

I'm just making a full code check. I'll let you know the result.

Regards

@slnj
Copy link
Author

slnj commented Feb 20, 2022

Hi Günter,
Great!!
All the errors regarding this issue disappeared.
I'll open separate issues for the remaining ones as they are not related to enable_if

Thanks!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants