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

Work around Core Issue 2174 #177

Conversation

matthew-reynolds
Copy link
Contributor

Our usage of deduced return types of in-template-class non-template friend
functions appears to be broken under the Green Hills (GHS) MULTI toolchain
(v2022.1). I believe this is an issue in the standard that was addressed for
C++17, but the resolution was not backported in C++14. GHS appears to have
not fully implemented the resolution, even in C++17 mode.

The GHS compiler, in C++17 mode, fails with the following error:

"external/au/au/quantity.hh", line 406: error #3171: cannot deduce the return
          type of function "au::operator+(au::Quantity<au::Celsius, float>,
          au::Quantity<au::Celsius, float>)" (declared at line 310); it has
          not been defined
      template <typename U = UnitT, typename = std::enable_if_t<IsUnitlessUnit<U>::value>>
                                                                ^
          detected during:
            instantiation of "bool au::QuantityPoint<UnitT,
                      RepT>::should_enable_implicit_construction_from<OtherUnit
                      ,OtherRep>() [with UnitT=au::Celsius, RepT=float,
                      OtherUnit=au::Celsius, OtherRep=float]" at line 95 of
                      "external/au/au/quantity_point.hh"

Prior to C++17, it was not clear what point the definition of a non-template
friend function within a class template was instantiated (And by consequence, at
what point the return type of the function is deduced). The C++14 standard
suggests that instantiation of a class template that contains a friend function
definition instantiates the declaration of the friend function, but not the
definition. The definition is not instantiated until it is odr-used. This is
particularly problematic for return type deduction in constexpr contexts where
there is no odr-use. I believe this is the case in
au::QuantityPoint::should_enable_implicit_construction_from(), where I
encounter the error with GHS.

Core Issue 2174 [1] seems to have addressed this for C++17, but GHS continues to
struggle with it even in C++17 mode. Given the fact that au targets C++14 but
this defect report was not backported to C++14, and GHS continues to struggle
even in C++17 mode, I am proposing we just remove our usage of deduced return
types for in-template-class definitions of non-template friend functions.

Note that the surrounding in-template-class friend functions are unaffected,
as they are either already using explicit return types or are function
templates.

See also this discussion from 2013, when deduced return types were introduced:

[1] https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2174

…ions

Our usage of deduced return types of in-class non-template friend functions
appears to be broken under the Green Hills (GHS) MULTI toolchain (v2022.1). I
believe this is an issue in the standard that was addressed for C++17, but the
clarification was not backported in C++14. GHS appears to have not fully
implemented the resolution, even in C++17 mode.

The GHS compiler, in C++17 mode, fails with the following error:

```
"external/au/au/quantity.hh", line 406: error #3171: cannot deduce the return
          type of function "au::operator+(au::Quantity<au::Celsius, float>,
          au::Quantity<au::Celsius, float>)" (declared at line 310); it has
          not been defined
      template <typename U = UnitT, typename = std::enable_if_t<IsUnitlessUnit<U>::value>>
                                                                ^
          detected during:
            instantiation of "bool au::QuantityPoint<UnitT,
                      RepT>::should_enable_implicit_construction_from<OtherUnit
                      ,OtherRep>() [with UnitT=au::Celsius, RepT=float,
                      OtherUnit=au::Celsius, OtherRep=float]" at line 95 of
                      "external/au/au/quantity_point.hh"
```

Prior to C++17, it was not clear what point the definition of a non-template
friend function within a class template was instantiated (And by consequence, at
what point the return type of the function is deduced). The C++14 standard
suggests that instantiation of a class template that contains a friend function
definition instantiates the _declaration_ of the friend function, but not the
_definition_. The definition is not instantiated until it is odr-used. This is
particularly problematic for return type deduction in `constexpr` contexts where
there is no odr-use. I believe this is the case in
`au::QuantityPoint::should_enable_implicit_construction_from()`, where I
encounter the error with GHS.

Core Issue 2174 [1] seems to have addressed this for C++17, but GHS continues to
struggle with it even in C++17 mode. Given the fact that `au` targets C++14 but
this defect report was not backported to C++14, and GHS continues to struggle
even in C++17 mode, I am proposing we remove our usage of deduced return types
for in-template-class definitions of non-template friend functions.

[1] https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2174

See also this discussion from 2013, when deduced return types were introduced:
 - https://stackoverflow.com/a/19325983
 - https://groups.google.com/a/isocpp.org/g/std-discussion/c/jahm7_tIN1Q
@CLAassistant
Copy link

CLAassistant commented Oct 2, 2023

CLA assistant check
All committers have signed the CLA.

@chiphogg chiphogg self-requested a review October 7, 2023 02:45
@chiphogg chiphogg added the release notes: 🐛 lib (bugfix) PR fixing a defect in the library code label Oct 7, 2023
Copy link
Contributor

@chiphogg chiphogg left a comment

Choose a reason for hiding this comment

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

It's too bad we have to get rid of the deduced return type, but you make a compelling argument. At least we're not adding our first macro. 🙂

Please find a shorter title (50 characters) to comply with best practices.

And thank you for the really exemplary explanation in the PR summary!

Once you get the CLA signed, the title edited, and the internal build issues sorted out, I plan to land it.

@matthew-reynolds matthew-reynolds changed the title Do not use deduced return type for in-template-class non-template friend functions Resolve Core Issue 2174 Oct 10, 2023
@chiphogg chiphogg changed the title Resolve Core Issue 2174 Work around Core Issue 2174 Oct 10, 2023
@chiphogg chiphogg merged commit a575880 into aurora-opensource:main Oct 10, 2023
9 checks passed
@matthew-reynolds matthew-reynolds deleted the matthew-reynolds/non-template-friend-function-deduced-return-type branch October 10, 2023 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: 🐛 lib (bugfix) PR fixing a defect in the library code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants