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.

Sign up for GitHub

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

Jump to bottom

BIR #2702

Merged
merged 16 commits into from
Oct 26, 2023
Merged

BIR #2702

merged 16 commits into from
Oct 26, 2023

Conversation

jdupak
Copy link
Contributor

@jdupak jdupak commented Oct 19, 2023 •
edited
Loading

Depends on umberged PR's (hence the noise). #2689 #2697

Initial structure for borrowchecking IR.

@jdupak jdupak changed the title Borrowck init BIR Oct 19, 2023
@jdupak
Copy link
Contributor Author

jdupak commented Oct 19, 2023

@CohenArthur

@jdupak jdupak force-pushed the borrowck-init branch 3 times, most recently from 1ce8396 to 42ad1ad Compare October 20, 2023 12:07
@jdupak
Copy link
Contributor Author

jdupak commented Oct 20, 2023

Fixed all commits to compile with gcc48.

dkm
dkm reviewed Oct 20, 2023
View reviewed changes
gcc/rust/checks/errors/borrowck/bir-design-notes.md
@@ -0,0 +1,188 @@
# Borrow-checker IR (BIR) design notes
Copy link
Member

Choose a reason for hiding this comment

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

should we have all these doc files in a doc/ subdir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am fine with either variant. If docs/ is more consistent, we can go with that.

dkm
dkm reviewed Oct 20, 2023
View reviewed changes
Copy link
Member

@dkm dkm left a comment

Choose a reason for hiding this comment

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

minor question

@jdupak jdupak marked this pull request as ready for review October 23, 2023 13:23
CohenArthur
CohenArthur reviewed Oct 23, 2023
View reviewed changes
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.

since the PR is very big, I will split my reviewing in a couple of passes as well. I've left some comments but I haven't finished reading through all of it - which I'll get to later in the day :) thank you, this is very impressive work! looking forward to understanding all of it!

gcc/po/gcc.pot Outdated Show resolved Hide resolved
gcc/rust/checks/errors/borrowck/rust-bir.h Show resolved Hide resolved
gcc/rust/checks/errors/borrowck/rust-bir.h Outdated Show resolved Hide resolved
gcc/rust/checks/errors/borrowck/rust-bir.h
Comment on lines +60 to +68
Kind kind;
// ASSIGNMENT: lhs
// SWITCH: switch_val
// StorageDead/StorageLive: place
// otherwise: <unused>
PlaceId place;
// ASSIGNMENT: rhs
// otherwise: <unused>
std::unique_ptr<AbstractExpr> expr;
Copy link
Member

Choose a reason for hiding this comment

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

so in Rust this would be something like this, correct?

enum Kind {
    Assignment { lhs: PlaceId, rhs: AbstractExpr },
    Switch { switch_val: PlaceId },
    StorageDead(PlaceId),
    StorageAlive(PlaceId),
    Goto,
    Return,
}

if that is the case, then I think a better API would be to have methods like get_lhs(), get_rhs() or get_switch_value() which would rust_assert() that we have the proper kind. does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so in Rust this would be something like this, correct?

Yes

if that is the case, then I think a better API would be to have methods like get_lhs(), get_rhs() or get_switch_value() which would rust_assert() that we have the proper kind. does that make sense?

Sure, sounds good. I am not convinced it is actually useful here, but it might as the checks get more complicated.

gcc/rust/checks/errors/borrowck/rust-bir.h Outdated Show resolved Hide resolved
gcc/rust/checks/errors/borrowck/rust-bir.h Outdated Show resolved Hide resolved
gcc/rust/checks/errors/borrowck/rust-borrow-checker.cc Show resolved Hide resolved
gcc/rust/checks/errors/borrowck/rust-function-collector.h Outdated Show resolved Hide resolved
gcc/rust/checks/errors/borrowck/rust-function-collector.h Show resolved Hide resolved
@jdupak jdupak force-pushed the borrowck-init branch 6 times, most recently from 8f3a647 to 10bd831 Compare October 25, 2023 17:20
jdupak added 13 commits October 25, 2023 20:06
@jdupak
borrowck: Add initial structure for borrowchecking
0b3d408 
gcc/rust/ChangeLog:

	* Make-lang.in: Build borrowck.
	* checks/errors/borrowck/rust-borrow-checker.cc: New file.
	* checks/errors/borrowck/rust-borrow-checker.h: New file.
	* checks/errors/borrowck/rust-function-collector.h: New file.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Add CLI option for borrowck
193cb2e 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-borrow-checker.cc (BorrowChecker::BorrowChecker): Opt dump.
	(BorrowChecker::go): Opt dump.
	* checks/errors/borrowck/rust-borrow-checker.h (class BorrowChecker): Opt dump.
	* lang.opt: Add compile until borrowcheck.
	* rust-session-manager.cc (Session::enable_dump): Add BIR.
	(Session::compile_crate): Handle new options.
	* rust-session-manager.h (struct CompileOptions): Add BIR.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Execute only with CLI flag
9f20320 
gcc/rust/ChangeLog:

	* lang.opt: CLI flag.
	* rust-session-manager.cc (Session::compile_crate): Guard execution.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Create Borrow-checker IR (BIR)
d7bcc5e 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-borrow-checker.cc: Include to compile new code.
	* checks/errors/borrowck/rust-bir-place.h: New file.
	* checks/errors/borrowck/rust-bir-visitor.h: New file.
	* checks/errors/borrowck/rust-bir.h: New file.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Create BIR builders (visitors)
4656475 
gcc/rust/ChangeLog:

	* Make-lang.in: Compile BIR expr visitor.
	* checks/errors/borrowck/rust-borrow-checker.cc (BorrowChecker::go): Use BIR builder.
	* rust-session-manager.cc (Session::compile_crate): Run borrow checker.
	* checks/errors/borrowck/rust-bir-builder-expr-stmt.cc: New file.
	* checks/errors/borrowck/rust-bir-builder-expr-stmt.h: New file.
	* checks/errors/borrowck/rust-bir-builder-internal.h: New file.
	* checks/errors/borrowck/rust-bir-builder-lazyboolexpr.h: New file.
	* checks/errors/borrowck/rust-bir-builder-pattern.h: New file.
	* checks/errors/borrowck/rust-bir-builder-struct.h: New file.
	* checks/errors/borrowck/rust-bir-builder.h: New file.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: BIR dump
13e67dd 
gcc/rust/ChangeLog:

	* Make-lang.in: Build BIR dump.
	* checks/errors/borrowck/rust-borrow-checker.cc (mkdir_wrapped): Cross-platform mkdir.
	(dump_function_bir): Save dump to file.
	(BorrowChecker::go): Run dump during borrowck.
	* checks/errors/borrowck/rust-bir-dump.cc: New file.
	* checks/errors/borrowck/rust-bir-dump.h: New file.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Dump: proper comma separation
42000a1 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-bir-dump.cc (Dump::go): Use new print.
	(print_comma_separated): Comma separation print.
	(Dump::visit): Use new print.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Dump: simplify cfg
c304940 
Add option to simplify cfg for better readability. Off by default.

gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-bir-dump.cc (simplify_cfg): Simplify cfg logic.
	(Dump::go): Run simplify cfg.
	* checks/errors/borrowck/rust-bir-dump.h: Option to simplify cfg.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Dump improve jumps
d70becd 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-bir-dump.cc (Dump::go): Improve jumps dump.
	(Dump::visit): Improve jumps dump.
	* checks/errors/borrowck/rust-bir-dump.h (class Dump): Improve jumps dump.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: BIR: handle break
f13ab05 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-bir-builder-expr-stmt.cc (ExprStmtBuilder::visit): Push ctx.
	(ExprStmtBuilder::setup_loop): Common loop infractructure setup.
	* checks/errors/borrowck/rust-bir-builder-expr-stmt.h: Loop ctx.
	* checks/errors/borrowck/rust-bir-builder-internal.h (struct BuilderContext): Loop ctx.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Dump: handle infinite loops
917b6bd 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-bir-dump.cc (simplify_cfg): Detech infinite loops.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: BIR continue
fa9b72f 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-bir-builder-expr-stmt.cc (ExprStmtBuilder::visit): Continue.
	(ExprStmtBuilder::setup_loop): Continue.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Make goto explicit.
4d6af23 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-bir-builder-expr-stmt.cc (ExprStmtBuilder::visit): Use goto.
	* checks/errors/borrowck/rust-bir-builder-internal.h: Explicit goto.
	* checks/errors/borrowck/rust-bir-builder-lazyboolexpr.h: Explicit goto.

Signed-off-by: Jakub Dupak <[email protected]>
jdupak added 3 commits October 25, 2023 20:06
@jdupak
borrowck: Docs
a46aec1 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/bir-design-notes.md: New file.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Dev notes
c29c900 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/dev-notes.md: New file.

Signed-off-by: Jakub Dupak <[email protected]>
@jdupak
borrowck: Refactor and BIR improvements
0798bc5 
gcc/rust/ChangeLog:

	* checks/errors/borrowck/rust-bir-builder-expr-stmt.cc (ExprStmtBuilder::setup_loop): Move.
	(ExprStmtBuilder::get_label_ctx): Move.
	(ExprStmtBuilder::get_unnamed_loop_ctx): Moved.
	(ExprStmtBuilder::visit): BIR improvements.
	* checks/errors/borrowck/rust-bir-builder-expr-stmt.h: Refactor.
	* checks/errors/borrowck/rust-bir-builder-internal.h (class LifetimeResolver):
	Refactor.
	(struct BuilderContext): Move.Refactor.
	(optional_from_ptr): Map on null ptr.
	* checks/errors/borrowck/rust-bir-builder-lazyboolexpr.h (class LazyBooleanExprBuilder):
	Refactor.
	* checks/errors/borrowck/rust-bir-builder-pattern.h: Refactor.
	* checks/errors/borrowck/rust-bir-builder-struct.h (class StructBuilder): Refactor.
	* checks/errors/borrowck/rust-bir-builder.h: Refactor.
	* checks/errors/borrowck/rust-bir-dump.cc (Dump::go): Refactor.
	(Dump::visit): Refactor.
	(Dump::visit_place): Refactor.
	(Dump::visit_move_place): Refactor.
	(Dump::visit_lifetime): Refactor.
	* checks/errors/borrowck/rust-bir-dump.h: Refactor.
	* checks/errors/borrowck/rust-bir-place.h: Refactor.

Signed-off-by: Jakub Dupak <[email protected]>
CohenArthur
CohenArthur approved these changes Oct 26, 2023
View reviewed changes
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.

amazing work, thank you! I'm excited to merge this :)

@CohenArthur CohenArthur added this pull request to the merge queue Oct 26, 2023
Merged via the queue into Rust-GCC:master with commit 2a1a373 Oct 26, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkm dkm dkm left review comments

@tschwinge tschwinge tschwinge left review comments

@CohenArthur CohenArthur CohenArthur approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jdupak @tschwinge @dkm @CohenArthur