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

C++: Support concept id expressions #18366

Merged
merged 7 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2,382 changes: 2,382 additions & 0 deletions cpp/downgrades/7eeff19bf7c89a350d3e43516a33c98a270cb057/old.dbscheme

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
description: Support concept id expressions
compatibility: full
concept_instantiation.rel: delete
is_type_constraint.rel: delete
5 changes: 5 additions & 0 deletions cpp/ql/lib/change-notes/2024-12-24-concept-id.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
category: feature
---
* A new class `ConceptIdExpr` was introduced, which represents C++20 concept id expressions.

103 changes: 102 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/Concept.qll
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,107 @@ class NestedRequirementExpr extends Expr, @nested_requirement {

/**
* A C++ concept id expression.
*
* For example, if:
* ```cpp
* template<typename T, T X> concept C = ...;
* ...
* requires { C<int, 1>; };
* ```
* then `C<int, 1>` is a concept id expression that refers to
* the concept `C`.
*/
class ConceptIdExpr extends RequirementExpr, @concept_id {
override string toString() { result = "concept<...>" }
override string toString() {
result = this.getConcept().getName() + "<...>"
or
// The following is for backward compatibility with databases created with
// CodeQL 2.19.3, 2.19.4, and 2.20.0. Those databases include concept id
// expressions, but do not include concept template information.
not exists(this.getConcept()) and
result = "concept<...>"
}

override string getAPrimaryQlClass() { result = "ConceptIdExpr" }

/**
* Holds if the concept id is used as a type constraint.
*
* In this case, the first template argument is implicit.
*/
predicate isTypeConstraint() { is_type_constraint(underlyingElement(this)) }

/**
* Gets the concept this concept id refers to.
*/
Concept getConcept() { concept_instantiation(underlyingElement(this), unresolveElement(result)) }

/**
* Gets a template argument passed to the concept.
*/
final Locatable getATemplateArgument() { result = this.getTemplateArgument(_) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observation: It feels like we need a new return type class TemplateArgument here, but I see that this code follows the examples elsewhere in the library, so don't fix this now.


/**
* Gets the kind of a non-type template argument passed to the concept.
*/
final Locatable getATemplateArgumentKind() { result = this.getTemplateArgumentKind(_) }

/**
* Gets the `i`th template argument passed to the concept.
*
* For example, if:
* ```cpp
* template<typename T, T X> concept C = ...;
* ...
* requires { C<int, 1>; };
* ```
* then `getTemplateArgument(0)` yields `int`, and `getTemplateArgument(1)`
* yields `1`.
*
* If the concept id is a type constraint, then `getTemplateArgument(0)`
* will not yield a result.
*/
final Locatable getTemplateArgument(int index) {
Dismissed Show dismissed Hide dismissed
if exists(this.getTemplateArgumentValue(index))
then result = this.getTemplateArgumentValue(index)
else result = this.getTemplateArgumentType(index)
}

/**
* Gets the kind of the `i`th template argument value passed to the concept.
*
* For example, if:
* ```cpp
* template<typename T, T X> concept C = ...;
* ...
* requires { C<int, 1>; };
* ```
* then `getTemplateArgumentKind(1)` yields `int`, and there is no result for
* `getTemplateArgumentKind(0)`.
*/
final Locatable getTemplateArgumentKind(int index) {
Dismissed Show dismissed Hide dismissed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused here. Why is the word Kind used, and what is a "kind"? I note that this predicate always yields a Type, so should this be the return type?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Could the name Type getTemplateValueArgumentType() be suitable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is consistent with Declaration::getTemplateArgumentKind/1 and FunctionCall::getTemplateArgumentKind/1.

I could do a follow-up where I change all of them, and deprecate all the getTemplateArgumentKind. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like getNonTypeTemplateArgumentType is probably most accurate (as "value" is just an internal name).

exists(this.getTemplateArgumentValue(index)) and
result = this.getTemplateArgumentType(index)
}

/**
* Gets the number of template arguments passed to the concept.
*/
final int getNumberOfTemplateArguments() {
result = count(int i | exists(this.getTemplateArgument(i)))
}

private Type getTemplateArgumentType(int index) {
exists(int i | if this.isTypeConstraint() then i = index - 1 else i = index |
concept_template_argument(underlyingElement(this), i, unresolveElement(result))
Copy link
Contributor Author

@jketema jketema Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that there's a trade-off in the design here: I follow what we do for the arguments in template instantiations, as concept ids are similar to those. Alternatively we could have added them as a kind of child expressions. The latter would have made the modifications to PrintAST somewhat simpler, but would have required custom code on the extractor-side, while in the current situation that shares code with template instantiations.

)
}

private Expr getTemplateArgumentValue(int index) {
exists(int i | if this.isTypeConstraint() then i = index - 1 else i = index |
concept_template_argument_value(underlyingElement(this), i, unresolveElement(result))
)
}
}

/**
Expand Down Expand Up @@ -187,4 +283,9 @@ class Concept extends Declaration, @concept_template {
* the constraint expression is `std::is_same<T, int>::value`.
*/
Expr getExpr() { result.getParent() = this }

/**
* Gets a concept id expression that refers to this concept
*/
ConceptIdExpr getAReferringConceptIdExpr() { this = result.getConcept() }
}
2 changes: 1 addition & 1 deletion cpp/ql/lib/semmle/code/cpp/Declaration.qll
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ class Declaration extends Locatable, @declaration {
*
* `Foo<int, 1> bar;`
*
* Will have `getTemplateArgument())` return `int`, and
* Will have `getTemplateArgument(0)` return `int`, and
* `getTemplateArgument(1)` return `1`.
*/
final Locatable getTemplateArgument(int index) {
Expand Down
95 changes: 91 additions & 4 deletions cpp/ql/lib/semmle/code/cpp/PrintAST.qll
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ private Declaration getAnEnclosingDeclaration(Locatable ast) {
or
result = ast.(Initializer).getDeclaration()
or
exists(ConceptIdExpr concept | ast = concept.getATemplateArgument() |
result = concept.getEnclosingFunction()
)
or
result = ast
}

Expand All @@ -107,6 +111,12 @@ private newtype TPrintAstNode =
TRequiresExprParametersNode(RequiresExpr req) {
shouldPrintDeclaration(getAnEnclosingDeclaration(req))
} or
TConceptIdExprArgumentsNode(ConceptIdExpr concept) {
shouldPrintDeclaration(getAnEnclosingDeclaration(concept))
} or
TConceptIdExprTypeArgumentNode(Type type, ConceptIdExpr concept, int childIndex) {
type = concept.getTemplateArgument(childIndex)
} or
TConstructorInitializersNode(Constructor ctor) {
ctor.hasEntryPoint() and
shouldPrintDeclaration(ctor)
Expand Down Expand Up @@ -357,6 +367,26 @@ class StringLiteralNode extends ExprNode {
override string getValue() { result = "\"" + escapeString(expr.getValue()) + "\"" }
}

/**
* A node representing a `ConceptIdExpr`.
*/
class ConceptIdExprNode extends ExprNode {
override ConceptIdExpr expr;

override PrintAstNode getChildInternal(int childIndex) {
result = super.getChildInternal(childIndex)
or
childIndex = -1 and
result.(ConceptIdExprArgumentsNode).getConceptIdExpr() = expr
}

override string getChildAccessorPredicateInternal(int childIndex) {
result = super.getChildAccessorPredicateInternal(childIndex)
or
childIndex = -1 and result = "<args>"
}
}

/**
* A node representing a `Conversion`.
*/
Expand Down Expand Up @@ -593,6 +623,63 @@ class InitializerNode extends AstNode {
}
}

/**
* A node representing the arguments of a `ConceptIdExpr`.
*/
class ConceptIdExprArgumentsNode extends PrintAstNode, TConceptIdExprArgumentsNode {
ConceptIdExpr concept;

ConceptIdExprArgumentsNode() { this = TConceptIdExprArgumentsNode(concept) }

final override string toString() { result = "" }

final override Location getLocation() { result = getRepresentativeLocation(concept) }

override PrintAstNode getChildInternal(int childIndex) {
exists(Locatable arg | arg = concept.getTemplateArgument(childIndex) |
result.(ConceptIdExprTypeArgumentNode).isArgumentNode(arg, concept, childIndex)
or
result.(ExprNode).getAst() = arg
)
}

override string getChildAccessorPredicateInternal(int childIndex) {
exists(this.getChildInternal(childIndex)) and
result = "getTemplateArgument(" + childIndex + ")"
}

/**
* Gets the `ConceptIdExpr` for which this node represents the parameters.
*/
final ConceptIdExpr getConceptIdExpr() { result = concept }
}

/**
* A node representing a type argument of a `ConceptIdExpr`.
*/
class ConceptIdExprTypeArgumentNode extends PrintAstNode, TConceptIdExprTypeArgumentNode {
Type type;
ConceptIdExpr concept;
int index;

ConceptIdExprTypeArgumentNode() { this = TConceptIdExprTypeArgumentNode(type, concept, index) }

final override string toString() { result = qlClass(type) + type.toString() }

final override Location getLocation() { result = getRepresentativeLocation(type) }

override AstNode getChildInternal(int childIndex) { none() }

override string getChildAccessorPredicateInternal(int childIndex) { none() }

/**
* Holds if `t` is the `i`th template argument of `c`.
*/
predicate isArgumentNode(Type t, ConceptIdExpr c, int i) {
type = t and concept = c and index = i
}
}

/**
* A node representing the parameters of a `Function`.
*/
Expand All @@ -611,7 +698,7 @@ class FunctionParametersNode extends PrintAstNode, TFunctionParametersNode {

override string getChildAccessorPredicateInternal(int childIndex) {
exists(this.getChildInternal(childIndex)) and
result = "getParameter(" + childIndex.toString() + ")"
result = "getParameter(" + childIndex + ")"
}

/**
Expand All @@ -638,7 +725,7 @@ class RequiresExprParametersNode extends PrintAstNode, TRequiresExprParametersNo

override string getChildAccessorPredicateInternal(int childIndex) {
exists(this.getChildInternal(childIndex)) and
result = "getParameter(" + childIndex.toString() + ")"
result = "getParameter(" + childIndex + ")"
}

/**
Expand All @@ -665,7 +752,7 @@ class ConstructorInitializersNode extends PrintAstNode, TConstructorInitializers

final override string getChildAccessorPredicateInternal(int childIndex) {
exists(this.getChildInternal(childIndex)) and
result = "getInitializer(" + childIndex.toString() + ")"
result = "getInitializer(" + childIndex + ")"
}

/**
Expand All @@ -692,7 +779,7 @@ class DestructorDestructionsNode extends PrintAstNode, TDestructorDestructionsNo

final override string getChildAccessorPredicateInternal(int childIndex) {
exists(this.getChildInternal(childIndex)) and
result = "getDestruction(" + childIndex.toString() + ")"
result = "getDestruction(" + childIndex + ")"
}

/**
Expand Down
5 changes: 5 additions & 0 deletions cpp/ql/lib/semmlecode.cpp.dbscheme
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,11 @@ concept_templates(
string name: string ref,
int location: @location_default ref
);
concept_instantiation(
unique int to: @concept_id ref,
int from: @concept_template ref
);
is_type_constraint(int concept_id: @concept_id ref);
concept_template_argument(
int concept_id: @concept ref,
int index: int ref,
Expand Down
Loading
Loading