Skip to content

Conversation

@randomPoison
Copy link
Collaborator

After #2594 we no longer use unimplemented! in the course, so I don't think we need to mention it in the macros slide.

@djmitche
Copy link
Collaborator

djmitche commented Feb 6, 2025

I do see it frequently in the wild, but I suppose it's easy enough for a student to look it up in the docs when they encounter it. What do you think @fw-immunant?

@djmitche djmitche requested a review from fw-immunant February 6, 2025 20:51
@fw-immunant
Copy link
Collaborator

Is this about unreachable! or unimplemented!?

@fw-immunant
Copy link
Collaborator

I think unreachable! is important to teach because Rust behaves differently here (by panicking safely) than C++'s unreachable which is an optimization hint and UB to execute.

@randomPoison randomPoison changed the title Remove unimplemented! from macros slide Remove unreachable! from macros slide Feb 11, 2025
@randomPoison
Copy link
Collaborator Author

Is this about unreachable! or unimplemented!?

Oh, I completely spaced and thought I was removing unimplemented!, but it is, in fact, unreachable! 😅. I'm still inclined to remove it since I don't think it adds much useful information for the students and, as @djmitche noted, it's easy to look up when encountered in the wild.

@fw-immunant would it be okay to move this into "More to Explore" in the speaker notes? There are other macros in std that could also be worth adding there as extra things to cover, time allowing.

@mgeisler
Copy link
Collaborator

@fw-immunant would it be okay to move this into "More to Explore" in the speaker notes? There are other macros in std that could also be worth adding there as extra things to cover, time allowing.

I like this idea: my gut feeling is that we have too much stuff overall, so anything non-essential that we can trim is a win. Adding a paragraph saying that there are more macros available would be useful.

@randomPoison
Copy link
Collaborator Author

I've added that note to the speaker notes along with a couple other macros that can be brought up in class if students want to know more.

Copy link
Collaborator

@djmitche djmitche 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 - now it just focuses on the macros used in the course!

@randomPoison randomPoison merged commit 241c28e into main Feb 26, 2025
37 checks passed
@randomPoison randomPoison deleted the legare/remove-unimplemented branch February 26, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants