Add `..` syntax to select a member function only by hsutter · Pull Request #1101 · hsutter/cppfront

hsutter · 2024-06-09T23:56:02Z

With this PR, x.f() is still UFCS, but x..f() will now find only a member function.

For now I like that the default is UFCS, but as we get more experience I'm open to changing the default so that . is member selection and .. is UFCS (which would be a breaking change, but a mechanical one).

Also: Remove the old version of get_declaration_of now that the new lookup seems stable.

With this commit, `x.f()` is still UFCS, but `x..f()` will now find only a member function. For now I like that the default is UFCS, but as we get more experience I'm open to changing the default so that `.` is member selection and `..` is UFCS (which would be a breaking change, but a mechanical one). Also: Remove the old version of `get_declaration_of` now that the new lookup seems stable.

jarzec · 2024-06-10T16:27:22Z

@hsutter It looks like builds/regression tests on some platforms fail due to warnings that become errors. Fixing the problems should not be difficult. Would you like some help with that?
In any case, I would suggest merging #1095 first as it fixes some pre-existing regression test issues.

jarzec · 2024-06-10T16:57:35Z

source/to_cpp1.h

        -> void
    {
+if (s.starts_with("..")) {
+    s == s;


This is causing "unused" warnings which lead to errors in the tests on some platforms. Is the if statement actually necessary?

I've just checked that this is the reason for all failures among the multi-platform build tests.
The failures in the regression tests seem to be mainly due to necessary line updates in error messages. This should be quick to fix, but first the s == s needs to be sorted out, as this might further change line numbers.

Ah, I should remove that... that was so I could set a debug breakpoint on 276 that would fire only when .. was happening, to investigate why .. wasn't yet lowering to . correctly.

OK, so let me get the tests running. I will create a pull request into this branch.
Please give me a couple of minutes.

Sorry, it took a bit longer than a couple of minutes, but here is the PR into this one: #1103.

EDIT: Not sure if I made this clear. #1103 updates the test files to make CI all green again. You can either merge this one and #1103 right after or merge #1103 into this branch first and then the whole thing into the main branch.

Thanks! This PR shouldn't affect any regression test output I think? The test generation should be identical, except only the one test I updated to add a .. test case which has expanded .cpp output but should not have any output changes because it doesn't print anything.

Re line# output: Usually the only kind of change that affects regression test line# output is when cpp2util.h changes cause the assertion line numbers to change for the handful of tests that a given older compiler doesn't fully support (typically GCC 10, sometimes Clang 12). So I don't think there should be line# regression test impact for this PR... I hope it's a pure extension!

Ah, true, you are right.

It seems some of the issues solved in #1103 are pre-existent (except the update of regression-tests/test-results/pure2-ufcs-member-access-and-chaining.cpp).
EDIT: Oh, even worse. Some of the issues fixed in #1103 are identical to those already merged, through. They would cause conflicts. I will remove those.

Done. The tests on #1103 will be failing until you merge it into this branch. You can also merge this one with failures on pure2-ufcs-member-access-and-chaining.cpp and then #1103 to fix those.

jarzec · 2024-06-10T21:29:33Z

source/sema.h

+        ////--------- START TEMPORARY REGRESSION TEST CODE FOR G_D_O OPTIMIZATION VERIFICATION ---------
+        ////  Now do a regression test violation check
+        ////
+        //if (
+        //    flag_internal_debug
+        //    && result_old != result
+        //    )
+        //{
+        //    std::cerr << "\n  Internal compiler error - see cppfront-ice-data.out for debug information\n\n";

-            auto out = std::ofstream{"cppfront-ice-data.out"};
+        //    auto out = std::ofstream{"cppfront-ice-data.out"};

-            out << "g_d_o arguments:\n";
-            out << "    " << static_cast<void const*>(&t) << " -> " << t.as_string_view() << " @ " << t.position().to_string()
-                << ", token order # " << t.get_global_token_order() << "\n";
-            out << "    look_beyond_current_function: " << std::boolalpha << look_beyond_current_function << "\n";
-            out << "    include_implicit_this:        " << std::boolalpha << include_implicit_this        << "\n";
+        //    out << "g_d_o arguments:\n";
+        //    out << "    " << static_cast<void const*>(&t) << " -> " << t.as_string_view() << " @ " << t.position().to_string()
+        //        << ", token order # " << t.get_global_token_order() << "\n";
+        //    out << "    look_beyond_current_function: " << std::boolalpha << look_beyond_current_function << "\n";
+        //    out << "    include_implicit_this:        " << std::boolalpha << include_implicit_this        << "\n";

-            out << "result_old:    "       << static_cast<void const*>(result_old) << "\n";
-            out << "result:        "       << static_cast<void const*>(result    ) << "\n\n";
+        //    out << "result_old:    "       << static_cast<void const*>(result_old) << "\n";
+        //    out << "result:        "       << static_cast<void const*>(result    ) << "\n\n";

-            debug_print(out);
+        //    debug_print(out);

-            exit(EXIT_FAILURE);
-        }
-        //--------- END TEMPORARY REGRESSION TEST CODE FOR G_D_O OPTIMIZATION VERIFICATION -----------
+        //    exit(EXIT_FAILURE);
+        //}
+        ////--------- END TEMPORARY REGRESSION TEST CODE FOR G_D_O OPTIMIZATION VERIFICATION -----------


I guess this was commented out for testing and can be removed now?
EDIT: If you do remove this code please don't merge #1103 yet. I will update it. This removal will likely require the error report lines in tests files to be updated again.

Suggested change

////--------- START TEMPORARY REGRESSION TEST CODE FOR G_D_O OPTIMIZATION VERIFICATION ---------

//// Now do a regression test violation check

////

//if (

// flag_internal_debug

// && result_old != result

// )

//{

// std::cerr << "\n Internal compiler error - see cppfront-ice-data.out for debug information\n\n";

auto out = std::ofstream{"cppfront-ice-data.out"};

// auto out = std::ofstream{"cppfront-ice-data.out"};

out << "g_d_o arguments:\n";

out << " " << static_cast<void const*>(&t) << " -> " << t.as_string_view() << " @ " << t.position().to_string()

<< ", token order # " << t.get_global_token_order() << "\n";

out << " look_beyond_current_function: " << std::boolalpha << look_beyond_current_function << "\n";

out << " include_implicit_this: " << std::boolalpha << include_implicit_this << "\n";

// out << "g_d_o arguments:\n";

// out << " " << static_cast<void const*>(&t) << " -> " << t.as_string_view() << " @ " << t.position().to_string()

// << ", token order # " << t.get_global_token_order() << "\n";

// out << " look_beyond_current_function: " << std::boolalpha << look_beyond_current_function << "\n";

// out << " include_implicit_this: " << std::boolalpha << include_implicit_this << "\n";

out << "result_old: " << static_cast<void const*>(result_old) << "\n";

out << "result: " << static_cast<void const*>(result ) << "\n\n";

// out << "result_old: " << static_cast<void const*>(result_old) << "\n";

// out << "result: " << static_cast<void const*>(result ) << "\n\n";

debug_print(out);

// debug_print(out);

exit(EXIT_FAILURE);

}

//--------- END TEMPORARY REGRESSION TEST CODE FOR G_D_O OPTIMIZATION VERIFICATION -----------

// exit(EXIT_FAILURE);

//}

////--------- END TEMPORARY REGRESSION TEST CODE FOR G_D_O OPTIMIZATION VERIFICATION -----------

Thanks! Yes I just forgot to remove that part. It shouldn't affect regression tests though. 👍

jcanizales · 2024-06-14T17:51:05Z

but x..f() will now find only a member function.

Hi, what is the use case this is addressing, if you don' t mind me asking?

gregmarr · 2024-06-14T18:01:11Z

but x..f() will now find only a member function.

Hi, what is the use case this is addressing, if you don' t mind me asking?

Quite a bit of discussion at #1004

jarzec · 2024-06-14T21:04:13Z

Is the PR ready to be merged?

hsutter · 2024-06-15T04:22:50Z

(deleted some comments with extraneous info)

@JarekGlobus wrote:

A relatively simple solution to running Linux on Windows without security concerns is using Docker containers. It is not difficult to set up such a solution. Docket is very popular so there is a lot of online material on using it.

I'll take a look. Thanks!

hsutter mentioned this pull request Jun 9, 2024

[SUGGESTION] Ability to disable UFCS both via cppfront and for regions in source code #1004

Closed

jarzec reviewed Jun 10, 2024

View reviewed changes

Remove some debug code

70fed4b

jarzec reviewed Jun 10, 2024

View reviewed changes

Finish removing old g_d_o paths

58002c9

JarekGlobus approved these changes Jun 13, 2024

View reviewed changes

jarzec approved these changes Jun 13, 2024

View reviewed changes

hsutter merged commit 9648191 into main Jun 15, 2024

Repository owner deleted a comment from JarekGlobus Jun 15, 2024

hsutter mentioned this pull request Jun 15, 2024

Fix tests after addition of .. support #1103

Merged

Conversation

hsutter commented Jun 9, 2024

Uh oh!

jarzec commented Jun 10, 2024

Uh oh!

jarzec Jun 10, 2024

Choose a reason for hiding this comment

Uh oh!

jarzec Jun 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hsutter Jun 10, 2024

Choose a reason for hiding this comment

Uh oh!

jarzec Jun 10, 2024

Choose a reason for hiding this comment

Uh oh!

jarzec Jun 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hsutter Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

jarzec Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

jarzec Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jarzec Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

jarzec Jun 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hsutter Jun 12, 2024

Choose a reason for hiding this comment

Uh oh!

jcanizales commented Jun 14, 2024

Uh oh!

gregmarr commented Jun 14, 2024

Uh oh!

jarzec commented Jun 14, 2024

Uh oh!

hsutter commented Jun 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jarzec Jun 10, 2024 •

edited

Loading

jarzec Jun 10, 2024 •

edited

Loading

jarzec Jun 12, 2024 •

edited

Loading

jarzec Jun 10, 2024 •

edited

Loading