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

[expr.add] Use better example #4778

Merged
merged 2 commits into from Sep 16, 2022

Conversation

languagelawyer
Copy link
Contributor

Related to #3614

@tkoeppe
Copy link
Contributor

tkoeppe commented Sep 25, 2021

Has the discussion in the linked issue been resolved?

@jensmaurer: any thoughts on this?

@jensmaurer
Copy link
Member

I think the main gist here is that using a pointer-to-base when actually pointing to a derived class is fine in most cases, but not in the array indexing case. Changing the example as suggested is losing that point.

@languagelawyer
Copy link
Contributor Author

languagelawyer commented Sep 27, 2021

I think the main gist here is that using a pointer-to-base when actually pointing to a derived class is fine in most cases, but not in the array indexing case.

Using a pointer-to-base when actually pointing to an object of derived class type is UB in most (if not all) cases (member access, member function call etc.).

Using pointer-to-base which points to the base subobject is fine in all cases (even in the array indexing case). What you can violate with such a pointer, is [expr.add]/4, not [expr.add]/6.

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 22, 2022

@jensmaurer I'm not sure what to do here. Should we move this example to p4 and edit the example in p6 like proposed? Or just keep the status quo?

@jensmaurer
Copy link
Member

I like the int / unsigned int example in p6, because it is mildly surprising.

I also like to retain the base / derived situation, but maybe it's worthwhile to formulate that in terms of "base class subobject" in p4, as a special case for when the p4.2 precondition isn't satisfied.

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 22, 2022

@languagelawyer Could you take another stab at this then?

@languagelawyer
Copy link
Contributor Author

@jensmaurer not sure what you meant, but I did this. Note, the Note is outside the itemize environment, unlike in the next paragraph.

@tkoeppe
Copy link
Contributor

tkoeppe commented Sep 8, 2022

@jensmaurer Could you please take another look here?

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

That looks reasonable to me.

If you can get @zygoloid to okay these changes, we can apply them right away, otherwise I'd like to have CWG assent.

source/expressions.tex Outdated Show resolved Hide resolved
source/expressions.tex Outdated Show resolved Hide resolved
@tkoeppe tkoeppe added the changes requested Changes to the wording or approach have been requested and not yet applied. label Sep 16, 2022
@tkoeppe
Copy link
Contributor

tkoeppe commented Sep 16, 2022

@languagelawyer could you please have a look?

@languagelawyer languagelawyer requested review from jensmaurer and zygoloid and removed request for zygoloid and jensmaurer September 16, 2022 16:55
@tkoeppe tkoeppe removed the changes requested Changes to the wording or approach have been requested and not yet applied. label Sep 16, 2022
@tkoeppe tkoeppe merged commit c70087a into cplusplus:main Sep 16, 2022
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.

None yet

4 participants