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

Reject const item with no value in impl context #2713

Merged
merged 11 commits into from
Nov 6, 2023

Conversation

P-E-P
Copy link
Member

@P-E-P P-E-P commented Oct 24, 2023

Add an AST validation visitor pass rejecting invalid rust code.

Fixes #2709

@P-E-P P-E-P added this to the GCC 14.1 release milestone Oct 24, 2023
@P-E-P P-E-P requested a review from CohenArthur October 24, 2023 14:05
@P-E-P P-E-P self-assigned this Oct 24, 2023
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

Looks good to me! Even if this is a big PR and a big change I think this is the way to go

gcc/rust/checks/errors/rust-ast-validation.h Show resolved Hide resolved
gcc/rust/checks/errors/rust-ast-validation.h Outdated Show resolved Hide resolved
@P-E-P P-E-P force-pushed the ast-validation branch 4 times, most recently from 3993eb1 to 53e16d9 Compare October 26, 2023 12:46
@P-E-P P-E-P marked this pull request as ready for review October 26, 2023 13:28
P-E-P added 11 commits October 27, 2023 10:43
Early passes visitors may encounter constant item without a value, this
is expected as the pass rejecting a constant without an expression is
done later during the ast validation.

gcc/rust/ChangeLog:

	* expand/rust-cfg-strip.cc (CfgStrip::visit): Add expr value check.
	* expand/rust-expand-visitor.cc (ExpandVisitor::visit): Likewise.
	* resolve/rust-early-name-resolver.cc (EarlyNameResolver::visit):
	Likewise.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
Add a new visitor to validate a given ast after the expansion pass.

gcc/rust/ChangeLog:

	* Make-lang.in: Add the new object file the list.
	* checks/errors/rust-ast-validation.cc: New file.
	* checks/errors/rust-ast-validation.h: New file.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
Add the ast validation and feature gating steps to the compile pipeline.

gcc/rust/ChangeLog:

	* lang.opt: Add the new compile options and update the enum values.
	* rust-session-manager.h (struct CompileOptions): Add the new steps to
	the enumeration.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
Add call to ast validation check, also add appropriate step to this pass
and the feature gating.

gcc/rust/ChangeLog:

	* rust-session-manager.cc (Session::compile_crate): Add call to ast
	validation.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
Associated const with no value that are not in trait impl are prohibited.

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::check): Launch
	check over the whole given crate.
	(ASTValidation::visit): Implement visitor for some members of the ast.
	* checks/errors/rust-ast-validation.h: Update some prototype according
	to implemented visitor functions. Also add a context tracker.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
The parameter type was used instead of the default value.

gcc/rust/ChangeLog:

	* ast/rust-ast-collector.cc (TokenCollector::visit): Check for presence
	of a type and use the default value instead of the type.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
Visitor pattern requires a getter to children using a mutable reference.

gcc/rust/ChangeLog:

	* ast/rust-ast.h: Add some missing mutable reference getters.
	* ast/rust-expr.h: Likewise.
	* ast/rust-item.h: Likewise.
	* ast/rust-path.h: Likewise.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
This will allow us to derive other visitors from it and overload only a
few selected visit methods.

gcc/rust/ChangeLog:

	* Make-lang.in: Add the new visitor object file.
	* ast/rust-ast-visitor.h (class DefaultASTVisitor): Create the default
	visitor class.
	* ast/rust-ast.h: Add a new reference getter for visitor pattern.
	* ast/rust-ast-visitor.cc: New file.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
This visitor is intended to be used by other visitors that require
context at some point for a given item.

gcc/rust/ChangeLog:

	* ast/rust-ast-visitor.cc (ContextualASTVisitor::visit): Add multiple
	context saving calls.
	* ast/rust-ast-visitor.h (class DefaultASTVisitor): Make visit
	functions virtual.
	(class ContextualASTVisitor): Add a stack like container for the
	current context chain.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
Use the new contextual ast visitor to reduce the amount of code in the
ast validation visitor.

gcc/rust/ChangeLog:

	* checks/errors/rust-ast-validation.cc (ASTValidation::visit): Adapt
	the call to the new visit functions.
	(ASTValidation::check): Launch the parent class visitor root function.
	* checks/errors/rust-ast-validation.h (class ASTValidation): Inherit
	from the contextual visitor.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
This new regression test highlight the fixed behavior for 2709.

gcc/testsuite/ChangeLog:

	* rust/compile/issue-2709.rs: New test.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

LGTM

@CohenArthur CohenArthur added this pull request to the merge queue Nov 6, 2023
Merged via the queue into Rust-GCC:master with commit 2c862b9 Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

ICE in impl const items without value
2 participants