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

[BUG] Type-scope object alias can't have deduced type or use CTAD #700

Open
JohelEGP opened this issue Sep 24, 2023 · 11 comments · May be fixed by #706
Open

[BUG] Type-scope object alias can't have deduced type or use CTAD #700

JohelEGP opened this issue Sep 24, 2023 · 11 comments · May be fixed by #706
Labels
bug Something isn't working

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented Sep 24, 2023

Title: Can't use CTAD for type-scope object alias.

Minimal reproducer (https://cpp2.godbolt.org/z/63occKT7x):

#include <array>
arr0: std::array == :std::array = (0);; // OK.
t: @struct type = {
  arr1: std::array == :std::array = (0);; // error.
}
main: () = { }
Commands:
cppfront main.cpp2
clang++17 -std=c++23 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -Werror=unused-result -I . main.cpp

Expected result:

A definitely not-incomplete object type
to be defined in the class' body.

  public: static constexpr std::array arr1 = std::array{0};

Actual result and error:

//=== Cpp2 type definitions and function declarations ===========================
  public: static const std::array arr1;   // error.
//=== Cpp2 function definitions =================================================
  inline constexpr std::array t::arr1 = std::array{0};
Cpp2 lowered to Cpp1:
//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"


class t;
  

//=== Cpp2 type definitions and function declarations ===========================

#include <array>
std::array inline constexpr arr0 = std::array{0};// OK.
class t {
  public: static const std::array arr1;   // error.
};
auto main() -> int;


//=== Cpp2 function definitions =================================================


  inline constexpr std::array t::arr1 = std::array{0};

auto main() -> int{}
Output:
main.cpp2:4:35: error: declaration of variable 'arr1' with deduced type 'const std::array' requires an initializer
    4 |   public: static const std::array arr1;   // error.
      |                                   ^
1 error generated.

See also:

@JohelEGP JohelEGP added the bug Something isn't working label Sep 24, 2023
@JohelEGP
Copy link
Contributor Author

JohelEGP commented Sep 24, 2023

Title: Type-scope object alias can't have deduced type.

Description:

This is what I used before commit b589f5d.

Minimal reproducer (https://cpp2.godbolt.org/z/jhnjWPY76):

#include <array>
arr0 :== :std::array = (0);; // OK.
t: @struct type = {
  arr1 :== :std::array = (0);; // error.
}
main: () = { }
Commands:
cppfront main.cpp2
clang++17 -std=c++23 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -Werror=unused-result -I . main.cpp

Expected result:

  public: static const auto arr1 = std::array{0};

Actual result and error:

//=== Cpp2 type definitions and function declarations ===========================
  public: static const auto arr1;// error.
//=== Cpp2 function definitions =================================================
  inline constexpr auto t::arr1 = std::array{0};
Cpp2 lowered to Cpp1:
//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"


class t;
  

//=== Cpp2 type definitions and function declarations ===========================

#include <array>
auto inline constexpr arr0 = std::array{0};// OK.
class t {
  public: static const auto arr1;// error.
};
auto main() -> int;


//=== Cpp2 function definitions =================================================


  inline constexpr auto t::arr1 = std::array{0};

auto main() -> int{}
Output:
main.cpp2:4:29: error: declaration of variable 'arr1' with deduced type 'const auto' requires an initializer
    4 |   public: static const auto arr1;// error.
      |                             ^
1 error generated.

@JohelEGP JohelEGP changed the title [BUG] Can't use CTAD for type-scope object alias [BUG] Type-scope object alias can't have deduced type or use CTAD Sep 24, 2023
@JohelEGP
Copy link
Contributor Author

One limitation of first declaring and then defining a type-scope object alias
is that implementations inconsistently accept their use as constexpr (tested with #703).
Defining in the class body always works: https://compiler-explorer.com/z/vabrK137E.
Otherwise, MSVC, Clang, and GCC give different results: https://compiler-explorer.com/z/zcdabTE4T.
For completeness, here's the previous one, but at namespace scope: https://compiler-explorer.com/z/rsabsnjGK.
(I'm polling their correctness at https://cpplang.slack.com/archives/C21PKDHSL/p1695693565690049).

@JohelEGP
Copy link
Contributor Author

I think that means that a templated @enum is bound to fail (on Clang and MSVC)
when you need an enumerator during constant evaluation.
I tried to actually test it, but a type-scope @enum fails: https://cpp2.godbolt.org/z/eheWhYYPz.

machine: @struct <Engine: type> type = {
  states: @enum = {
    on;
    off;
  }
}
main: () = { }
cppfront: source/cppfront.cpp:1870: void cpp2::cppfront::emit(const cpp2::compound_statement_node&, const cpp2::function_prolog&, const std::vector<std::__cxx11::basic_string<char> >&): Assertion `!current_functions.empty()' failed.
Program terminated with signal: SIGSEGV
Compiler returned: 139

@JohelEGP
Copy link
Contributor Author

Looks like the limitation was known.
From the linked SO: https://stackoverflow.com/a/32134757.

@hsutter
Copy link
Owner

hsutter commented Sep 26, 2023

Interim ack: In the meantime I'll add a diagnostic for an object alias in a nested type, which isn't currently supported. Thanks!

@JohelEGP

This comment was marked as off-topic.

@JohelEGP
Copy link
Contributor Author

Interim ack: In the meantime I'll add a diagnostic for an object alias in a nested type, which isn't currently supported. Thanks!

I think that's incidentally fixed by #703.

It actually doesn't.
But it's the same issue as #704.
Except that #706 fixes it for non-@enum aliases by defining them in the class body.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Oct 1, 2023

(I'm polling their correctness at https://cpplang.slack.com/archives/C21PKDHSL/p1695693565690049).

According to Brian Bi at https://cpplang.slack.com/archives/C21PKDHSL/p1696124673621459?thread_ts=1695800824.887719&cid=C21PKDHSL:

I'm discussing it with folks on the reflector. It should be valid, even though Clang/MSVC don't accept it yet.

So it seems those are just compiler bugs.

@hsutter
Copy link
Owner

hsutter commented Oct 1, 2023

Thanks!

Looks like the limitation was known.
From the linked SO: https://stackoverflow.com/a/32134757.

Right, and because Clang 3.4 is so old I just assumed that by now it would have been fixed. But it seems it's a rare enough case that it hasn't. Thank you very much for following up on this!

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Oct 31, 2023

A definitely not-incomplete object type
to be defined in the class' body.

This is troublesome to determine.
Heuristics can only get you so far.
For example, #706 can't solve this:

t: @struct type = {
  a: std::optional<t>   == (); // Should be defined out of class.
  b: std::optional<* t> == (); // Can be defined in class.
}

The heuristics of #706 are that those are defined in class.
It gets worse the more indirections one introduces:

f: () -> std::optional<t>   == ();
g: () -> std::optional<* t> == ();
t: @struct type = {
  a :== f(); // Should be defined out of class, but has a deduced type.
  b :== g(); // Can be defined in class (has a deduced type, so it should?).
}

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Oct 31, 2023

This is a limitation of Cppfront.

A simpler heuristic that should always works
is to define in class when the type is a placeholder.
So when you need to use CTAD,
or use the value in another member's signature,
you just need to use the placeholder type.

Done in #706.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants