[expr.const]/31 Small issue with example

[jakubjelinek]

https://eel.is/c++draft/expr.const#example-16

struct S6;
consteval {                                 // #1
  struct S7;                                // local class

  std::meta::define_aggregate(^^S7, {});    // error: consteval block #1 does not enclose itself,
                                            // but encloses S7

  consteval {                               // #2
    std::meta::define_aggregate(^^S6, {});  // error: consteval block #1 encloses
                                            // consteval block #2 but not S6

    std::meta::define_aggregate(^^S7, {});  // OK, consteval block #1 encloses both #2 and S7
  }
}

I think it would be better to add struct S8; below struct S7; and use S8 instead of S7 on the last define_aggregate line.
The problem is that the inner consteval block is evaluated first and successfully defines S7 to be complete with no non-static data members,
and only then is the first std::meta::define_aggregate(^^S7, {}); evaluated. And so it doesn't have just the error that it would create an injected declaration violating https://eel.is/c++draft/expr.const#31.2 but it also violates https://eel.is/c++draft/meta.reflection#define.aggregate-8.1
So, I think it would be better to show in the example something that has only one of the errors, not two where the latter one will cause no injected declaration being attempted at all.

[jakubjelinek]

@katzdm

[katzdm]

Sounds good. Concretely:

  struct S6;
  consteval {                                 // #1
    struct S7;                                // local class

    std::meta::define_aggregate(^^S7, {});    // error: consteval block #1 does not enclose itself,
                                              // but encloses S7

+  struct S8;
    consteval {                               // #2
      std::meta::define_aggregate(^^S6, {});  // error: consteval block #1 encloses
                                              // consteval block #2 but not S6

-     std::meta::define_aggregate(^^S7, {});  // OK, consteval block #1 encloses both #2 and S7
+     std::meta::define_aggregate(^^S8, {});  // OK, consteval block #1 encloses both #2 and S8
    }
  }

@jakubjelinek Is that what you were suggesting?

[jakubjelinek]

Yep.

[jensmaurer]

pull request welcome

[jakubjelinek]

@katzdm Completely OT, where does the standard specify whether the NSDMs introduced by injected declarations should be public: or private: or public:/private: depending on if the declaration uses struct or class?
https://godbolt.org/z/ro8YG9a1E shows both clang++ and g++ use public members, but not sure I understand where it is specified.

[jensmaurer]

@jakubjelinek , looks like "public" is missing from [meta.reflection.define.aggregate] p9.5.1. That's a bug.

@katzdm, are you opening an LWG issue for that?

[katzdm]

@katzdm Completely OT, where does the standard specify whether the NSDMs introduced by injected declarations should be public: or private: or public:/private: depending on if the declaration uses struct or class? https://godbolt.org/z/ro8YG9a1E shows both clang++ and g++ use public members, but not sure I understand where it is specified.

Embarrassingly, I don't think it is specified. I think it probably slipped by because an aggregate has all public members, but it doesn't say anywhere that the type is an aggregate 🤣

@jensmaurer Should I open an issue, or can this be editorial? Change is to [meta.reflection.define.aggregate]/9.5.1:

If N_K is ⊥, M_K is an unnamed bit-field. Otherwise, M_K is a public non-static data member whose name is the identifier determined by the character sequence encoded by N_K in UTF-8.

[jensmaurer]

@katzdm , please open an LWG issue.

[jakubjelinek]

Shouldn't even the unnamed bit-field be public ? is_accessible metafn certainly cares about it...

[katzdm]

Shouldn't even the unnamed bit-field be public ? is_accessible metafn certainly cares about it...

A bit tricky, since (as we know so well) unnamed bit-fields aren't members of the class - but you're right; the wording for is_accessible requires that we say something here.

There is some wording in [class.access] that refers to "the access of the member function" and to that "access" being "public". [class.access.general]/4 supports the idea that access control applies to declarations, and it therefore makes further sense to refer to a bit-field declaration "having access".

On the basis of those precedents, I suggest instead Modifying [meta.reflection.define.aggregate]/9.5 as follows:

For each r_K, there is a corresponding entity M_K with public access belonging to the class scope of D with the following properties:

@jensmaurer @jakubjelinek - If that works for you, I'll open the LWG issue.

[jakubjelinek]

LGTM

[katzdm]

https://cplusplus.github.io/LWG/issue4449