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

Inconsistencies in comments for ill-formed/undefined behavior examples #3126

Closed
burblebee opened this issue Aug 5, 2019 · 2 comments · Fixed by #3557
Closed

Inconsistencies in comments for ill-formed/undefined behavior examples #3126

burblebee opened this issue Aug 5, 2019 · 2 comments · Fixed by #3557
Assignees
Labels
big An issue causing a large set of changes, scattered across most of the text.

Comments

@burblebee
Copy link
Contributor

We're inconsistent in how we format/word comments in examples for ill-formed and undefined behavior code. The following cases show a few of the variations used:

utilities.tex:any_cast<string&>(y);                       // error; cannot
g<int>(0);        // error, substituting parameter type instantiates \tcode{A<int>}
S<char> s1;                     // error: no matching constructor
template<> enum A<char>::E : char { echar };        // ill-formed, \tcode{A<char>::E} was instantiated
struct Inner2;                // ill-formed, no diagnostic required
// ill-formed, no diagnostic required: the two expressions are functionally equivalent but not equivalent
void A<Y>::B<double>::mf2() { }       // ill-formed; \tcode{B<double>} is specialized but
template<class T> void f(T::R);         // ill-formed (no diagnostic required), attempt to declare
p->x.j = 99;                            // undefined: modifies a const subobject
int m = g(false);   // undefined behavior due to access of \tcode{x.n} outside its lifetime
C().C::~C();      // undefined behavior: temporary of type \tcode{C} destroyed twice
pb->f();          // undefined behavior, lifetime of \tcode{*pb} has ended

Suggestion: Change all to either of the following as appropriate:

  // ill-formed, no diagnostic required: <explanation>
  // error: <explanation>
  // undefined behavior: <explanation>

where : <explanation> can be omitted if it is clear from the surrounding text.

This does not apply in compatibility.tex, where we should stick with:
f(u8"text"); // ill-formed; previously well-formed

@jwakely
Copy link
Member

jwakely commented Aug 6, 2019

I don't think "undefined behavior" has much benefit over "undefined" in this context.

Although it's widely used in examples, strictly speaking "error" doesn't mean anything, those cases are ill-formed. Do we want to continue using "error" to clearly distinguish those from the "ill-formed, no diagnostic required" cases?

@jensmaurer jensmaurer changed the title Inconsistentcies in comments for ill-formed/undefined behavior examples Inconsistencies in comments for ill-formed/undefined behavior examples Aug 6, 2019
@jensmaurer jensmaurer added the decision-required A decision of the editorial group (or the Project Editor) is required. label Aug 6, 2019
@jensmaurer
Copy link
Member

Editorial teleconference: "undefined" -> "undefined behavior", use colon afterwards if the actual reason follows. Replace "ill-formed: explanation" with "error: explanation". Use "ill-formed, no diagnostic required: explanation". Use colon where it explains why the thing before the colon. Otherwise, leave unchanged for now.

@jensmaurer jensmaurer added decision-required A decision of the editorial group (or the Project Editor) is required. and removed decision-required A decision of the editorial group (or the Project Editor) is required. labels Sep 10, 2019
@jensmaurer jensmaurer added big An issue causing a large set of changes, scattered across most of the text. and removed decision-required A decision of the editorial group (or the Project Editor) is required. labels Sep 30, 2019
@jensmaurer jensmaurer self-assigned this Dec 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
big An issue causing a large set of changes, scattered across most of the text.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants