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

[dcl.enum] Merge duplicate normative paragraphs on redeclarations. #1827

Merged
merged 1 commit into from Nov 27, 2018

Conversation

jensmaurer
Copy link
Member

Fixes #1826.

@@ -2040,10 +2040,15 @@
If the \grammarterm{enum-key} is followed by a
Copy link
Member

Choose a reason for hiding this comment

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

Please add "in an enum-head or opaque-enum-declaration" after "enum-key". I think we may have duplicated this previously as a misguided and unclear attempt to address both cases separately.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, could we refactor opaque-enum-declaration to use enum-head-name, and reference that here? That would avoid confusion over whether this applies if there is an intervening attribute-specifier-seq.

@tkoeppe
Copy link
Contributor

tkoeppe commented Feb 13, 2018

@jensmaurer: actionable comments here

@jensmaurer
Copy link
Member Author

Actions performed. @zygoloid is next.

@zygoloid
Copy link
Member

Looks good, let's apply this after the Jacksonville motions.

@jensmaurer jensmaurer added the after-motions Pull request is to be applied after the pending edits from WG21 straw polls have been applied. label Feb 13, 2018
\grammarterm{nested-name-specifier}, the \grammarterm{enum-specifier} shall
If an \grammarterm{enum-head-name} contains a
\grammarterm{nested-name-specifier}, the latter
shall not begin with a \grammarterm{decltype-specifier} and
Copy link
Member

Choose a reason for hiding this comment

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

Was this decltype rule for opaque-enum-declarations present before (maybe implied by pre-existing wording)? I see we had such a rule for enum-heads.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, we have (unchanged from before):

If an \grammarterm{opaque-enum-declaration} contains
a \grammarterm{nested-name-specifier},
the declaration shall be an explicit specialization\iref{temp.expl.spec}.

and we have

11.3 [dcl.meaning]
The nested-name-specifier of a qualified declarator-id shall not begin with a decltype-specifier.

That seems close enough to me. (it seems we don't want redeclarations to use decltype to nominate the entity being redeclared. Which is totally reasonable.)

Copy link
Member

Choose a reason for hiding this comment

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

I agree that we don't want to allow decltype here, and indeed current implementations don't distinguish between the opaque enum declaration case and the regular enum case, but if this is a normative change (as it appears to be), we should fix that portion of this via the CWG issue process.

Copy link
Member Author

@jensmaurer jensmaurer Apr 2, 2018

Choose a reason for hiding this comment

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

Fixed by limiting the prohibition to the enum-head case.

@jensmaurer
Copy link
Member Author

Rebased.

@tkoeppe tkoeppe removed the after-motions Pull request is to be applied after the pending edits from WG21 straw polls have been applied. label Apr 3, 2018
@tkoeppe tkoeppe added the decision-required A decision of the editorial group (or the Project Editor) is required. label Apr 5, 2018
@tkoeppe
Copy link
Contributor

tkoeppe commented Apr 5, 2018

@zygoloid says:

I think what I'm most worried about here is that making the editorial change first turns the wording from "wrong but somewhat ambiguous" (it looks like we just forgot the rule) to "unambiguously wrong" (the difference between the two cases is clearly intentional), which I would expect to make it more likely for a user or implementer to get confused or do the wrong thing.

Let's run this by CWG then.

@tkoeppe tkoeppe added the cwg Issue must be reviewed by CWG. label Apr 5, 2018
@jensmaurer jensmaurer added not-editorial Issue is not deemed editorial; the editorial issue is kept open for tracking. and removed decision-required A decision of the editorial group (or the Project Editor) is required. not-editorial Issue is not deemed editorial; the editorial issue is kept open for tracking. labels Apr 20, 2018
@zygoloid zygoloid force-pushed the master branch 2 times, most recently from e3dbfe2 to 1a21a65 Compare July 7, 2018 23:19
@jensmaurer jensmaurer added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Oct 9, 2018
Also introduce the grammar non-terminal enum-head-name
to simplify the description.
@jensmaurer
Copy link
Member Author

Rebased.

@jensmaurer jensmaurer removed the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Oct 11, 2018
@jensmaurer
Copy link
Member Author

CWG in San Diego: Fine with the change.

http://wiki.edg.com/pub/Wg21sandiego2018/CoreWorkingGroup/cwg-ed1827.html

@jensmaurer jensmaurer removed the cwg Issue must be reviewed by CWG. label Nov 8, 2018
@zygoloid zygoloid merged commit 9a720cd into cplusplus:master Nov 27, 2018
@jensmaurer jensmaurer deleted the b17 branch November 27, 2018 11:35
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

3 participants