-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
C++: Introduce a new base class for template parameters #18308
Conversation
94e9b6e
to
fda2d75
Compare
This will enable us to support non-type template parameters, which we currently do not support, and error template parameters, which might become relevant in the `build-mode: none` context.
fda2d75
to
b7d1da8
Compare
--- | ||
category: feature | ||
--- | ||
* A new class `TemplateParameterBase` was introduced, which represents C++ non-type template parameters, type template parameters, and template template parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, this is currently a lie, as nothing represents non-type template parameters. I plan to fix this next.
@@ -1695,7 +1717,7 @@ class TemplateParameter extends UserType { | |||
* void foo(const Container<Elem> &value) { } | |||
* ``` | |||
*/ | |||
class TemplateTemplateParameter extends TemplateParameter { | |||
class TemplateTemplateParameter extends TypeTemplateParameter { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the description in the change note I thought that type template parameters and template template parameters where to different groups and wouldn't have expected TemplateTemplateParameter
to extend TypeTemplateParameter
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could pull that apart, but that will have quite some fallout in other places, and possibly for customers. So, I prefer to leave this as-is. Another observation is that template template parameters will still refer to (templated) types, so from that perspective it makes sense to have this layering, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks. Let's keep it like this then :)
cpp/ql/lib/semmle/code/cpp/Type.qll
Outdated
* In the example below, `T` and `I` are template parameters: | ||
* ``` | ||
* template <class T, int I> | ||
* class C { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be nice to add a template template parameter as well:
* In the example below, `T` and `I` are template parameters: | |
* ``` | |
* template <class T, int I> | |
* class C { }; | |
* In the example below, `T`, `I`, and `V` are template parameters: | |
* ``` | |
* template <class T, int I, template<typename> class V> | |
* class C { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in ccd3681
This will enable us to support non-type template parameters, which we currently do not support, and error template parameters, which might become relevant in the
build-mode: none
context.DCA was ran on the internal PR.
Pull Request checklist
All query authors
.qhelp
. See the documentation in this repository.Internal query authors only
.ql
,.qll
, or.qhelp
files. See the documentation (internal access required).