Skip to content

Commit

Permalink
Only convert type_of to the Cpp2 macro if unqualified
Browse files Browse the repository at this point in the history
  • Loading branch information
@hsutter
hsutter committed Oct 10, 2024
1 parent 3fe8d6f commit 0853341
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 3 deletions.
2 changes: 1 addition & 1 deletion regression-tests/test-results/version
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

cppfront compiler v0.8.0 Build 9A09:1351
cppfront compiler v0.8.0 Build 9A10:0946
Copyright(c) Herb Sutter All rights reserved

SPDX-License-Identifier: CC-BY-NC-ND-4.0
Expand Down
2 changes: 1 addition & 1 deletion source/build.info
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
"9A09:1351"
"9A10:0946"
6 changes: 5 additions & 1 deletion source/to_cpp1.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1699,7 +1699,11 @@ class cppfront
{
printer.print_cpp2("cpp2_"+n.to_string(), pos);
}
else if (n == "type_of") {
else if (
!is_qualified
&& n == "type_of"
)
{
printer.print_cpp2("CPP2_TYPEOF", pos);
}
else {
Expand Down

4 comments on commit 0853341

@JohelEGP
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm reading https://isocpp.org/files/papers/D2996R7.html.
It seems there's a type_of we can use UFCS on.
However, 0.type_of() currently lowers to CPP2_UFCS(CPP2_TYPEOF)(0) (https://cpp2.godbolt.org/z/jbjWjr8q6).
We should review these Cpp2 keywords and ensure their unqualified use in UFCS doesn't treat them specially.

@JohelEGP
Copy link
Contributor

Choose a reason for hiding this comment

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

But what about types, like u32?
arg.Type() is something we sold for UFCS.
I think 0.u32() should continue to have the same effects as its current lowering to CPP2_UFCS(cpp2::u32)(0) (https://cpp2.godbolt.org/z/YMx3G1133).
But what if the argument isn't 0,
but of a type with a u32 member,
or there's a u32 function object in scope,
or anything else that makes x..u32() or u32(x) valid?
It seems like we would need to enhance the UFCS macro
to conditionally try the cpp2::-qualified name as a last resort.

@JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented on 0853341 Oct 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Interestingly, I tried writing a type with a u32 member (https://cpp2.godbolt.org/z/d6cbjxhxz):

t: type = {
  u32: (this) -> void = { }
}
main: () = {
  x: t = ();
  x.u32();
  _ = x;
}

But it's declared as

public: auto cpp2::u32() const& -> void;

Is this intended?
Right now, the hypothetical x.u32() above does work as x.__identifier__u32().
So does declaring the member like that (https://cpp2.godbolt.org/z/3sWrazTvn):

t: type = {
  __identifier__u32: (this) -> void = { }
}
main: () = {
  x: t = ();
  x.__identifier__u32();
  _ = x;
}

So, maybe there's nothing to do?
Except diagnosing the use of those Cpp2 type keywords outside expressions and type-only contexts.
And maybe still enhancing the UFCS macro to add a static_assert suggesting the use of __identifier__.

@JohelEGP
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an easier solution.
For x.u32(), we could allow that to call a u32 member, or always treat that as cpp2::u32.
In the former case, we need to enhance the UFCS macro.
The latter case has the easier solution.
That is a separate UFCS macro, since we know we're dealing with a Cpp2 type keyword,
that just checks for validity and otherwise suggests the use of __identifier__.

Please sign in to comment.