Skip to content

Commit

Permalink
fix: recognize last uses (including of this) (#887)
Browse files Browse the repository at this point in the history 
* fix(sema): recognize last use immediately after declaration

* fix(sema): give function expression its own scope level

* fix(sema): consider use as function in UFCS call

* refactor(sema): remove stale comment

* test(sema): also add unit test for `move` parameter

* refactor(sema): avoid possible decrement beyond `begin`

* fix(sema): move `this` on last use

* test: update GCC 13 results

* test: add Clang 18 results

* test: add overloads with `that`

* test: add another case that doesn't work yet

* test: rename members to avoid shadowing

* fix(sema): move implicit `this` of member on last use

* test: remove commented cases that aren't last uses

* fix(sema): guard check for `move this` parameter once

* test: add case for destructor

* test: add missing GCC 13 result

* test: regenerate tests

* fix(sema): consider use as function in UFCS call

* fix(to_cpp1): type-scope variable initialization is not deferred

* refactor(util): add UFCS macros to move and forward

* fix(to_cpp1): emit last use of function in UFCS as part of macro name

* test: regenerate GCC 13 result

* fix(to_cpp1): exercise last use in range of for loop

* refactor(util): remove UFCS macros of invalid combinations

* refactor(to_cpp1): handle range of for loop specifically

* refactor(to_cpp1): consider the expression list range

* fix(sema): apply last use to call of member function

* fix(sema): do not move a member function

* fix(to_cpp1): do not move any member function

* test: remove non-problematic comment about double move

It just works, see <https://cpp1.godbolt.org/z/6fhjxrz65>.

* refactor: regenerate `reflect.h`

* refactor(sema): avoid decrementing before begin

* fix(sema): extend implicit else branch to containing statement

* fix(sema): increase scope to match an explicit else

* refactor(sema): move heuristic to not change ending depths

* fix(sema): pop consecutive implicit else branches

* refactor(parse): put implicit else statements in the actual else node

* test: add test cases that combine implicit and explicit access

* test: add unit test for all test cases as selections

* fix(reflect): fix initialization and regenerate

* fix(sema): apply sema phase to generated code

* test: regenerate results

* test: add test case for another limitation

* test: add another test case that fails

* fix(sema): improve heuristic to only move from last (implicit) `this`

* refactor(reflect): restore implicit `this` access

* fix(sema): do not stack an implicit else branch

* refactor: use "implicit scope" throughout

* fix(sema): use local `move this` to determine last use

* fix(sema): adjust condition for looking up to type

* fix(sema): use last move on called type-scope variable

* test: remove bogus comment

* fix(sema): correct unset `this.` case

* test: reference open issue

* test: add test cases not affected by the open issue

* test: clarify FIXME comment

* test: review the new tests

* perf: replace `std::map` with `mutable` data member

* refactor: add `symbol::get_final_position`

* refactor(to_cpp1): do not move single named return

* test: add new test cases to commented out test case

* test: add similar test case to callable case

* refactor(sema): rework some outdated whitespace

* test: regenerate results after "do not move single named return"

* feat(sema): diagnose unused variables in generate code

* fix(sema): consider variables in type scope declared later

* test: refactor new test to use `f_copy`

* refactor(sema): use member token

* refactor(sema): update comments

* refactor(sema): use the `this` parameter exclusively

* refactor(sema): update the comments

* refactor(sema): finish focusing on the implicit 'this'

* fix(to_cpp1): move returned uninitialized local

* fix(to_cpp1): move single named uninitialized return

* test: add case with member declared last

* refactor(sema): set condition for "at `this.`" correctly

* fix(sema): use the better lookup algorithm for this case

* fix(to_cpp1): stack current names only in namespaces

* fix(to_cpp1): stack current names of bases

* test: exercise forward in generated code

* test: add stand-in for `std::move_only_function`

* test: remove bogus test case

* refactor(to_cpp1): rename to `is_class_member_access`

* test: add test case showing limitations with base members

* test: actually exercise forward in generated code

* refactor(to_cpp1): reorder branch

* refactor(to_cpp1): remove outdated condition

* refactor(to_cpp1): rename to `is_class_member_access`

* fix: revert using the empty implicit else branch

Refs: e9cc033, 7f4a60f

* fix(sema): change algorithm to find last uses

* test: add test case for recursion logic

* refactor(sema): simplify condition for UFCS on member

* test: use `f_inout` and `f_copy` in earlier test cases

* test: enable commented out tests

* test: extend limitation unit test with alternative

* test: remove redundant explicit discards

* test: add more test cases for the new last use algorithm

* test: add missing `new<int>()`

* fix: regenerate `reflect.h`

* test: add test cases of discovered problems

* fix(sema): pop sibling branch conditions on found last use

* refactor(sema): localize variables

* fix(sema): recognize uses in iteration statements

* fix(sema): start from the last symbol, not past-the-end

* refactor(sema): add local type `pos_range`

* fix(sema): handle loops and (non) captures

* test: add similar case but without declaration

* test: regenerate results

* fix(reflect): update and regenerate `reflect.h`

* fix(sema): start for loop at its body

* refactor(sema): use `std::span`

* refactor(sema): register name deactivation at visit

* fix(sema): do not apply use to a hiding name

* fix(sema): skip hiding loop parameter

* test: revert `gcc-version.output`

* fix(sema): recognize use in _next-clause_

* test: add corner case

* fix(sema): recognize Cpp1 `using` declaration

* refactor(sema): avoid adding duplicate symbols

* refactor(sema): clarify similar members with comments

* refactor(sema): turn comment into code

* refactor(sema): modify local convenience variable

* refactor(sema): remove expression scope

* refactor(sema): use the right predicate

* refactor(sema): remove inactive, stale assertions

* refactor(sema): keep using a sentinel

* fix(sema): handle a nested true branch

* refactor(sema): revert whitespace change

* refactor(sema): fix `started_postfix_expression` simpler

* refactor(sema): revert stale fix of `scope_depth`

* refactor(sema): comment the need of `final_position` at hand-out

* refactor(to_cpp1): drop periods from comment

* refactor(to_cpp1): reorder arguments for better formatting

* refactor(to_cpp1): remove stale non-rvalue context

* refactor(to_cpp1): remove useless non-rvalue context

* refactor(to_cpp1): clarifiy comment with example

* test: regenerate gcc-13 results

Commit 4eef0da
actually worked around #746.

* test: regenerate results

* refactor(sema): resolve CI issues

* test: revert changes to gcc-13 result

* refactor: generalize to `identifier_sym::safe_to_move`

* test: fix implementation of `identity`

* refactor(sema): add scoped indices of uses

* refactor(sema): simplify names of activation entities

* fix(sema): do not mark non-captures as captures

* refactor(sema): rename to avoid verb prefix

According to <#231 (comment)>:
> to avoid dealing with English verb-to-adjective conventions

* fix(sema): disable implicit move unsequenced with another use

* fix(sema): consider _is-as-expression_ too

* fix(sema): check all parameters for use

* refactor(sema): remove wrong fix for UFCS issue

* Minor updates to compile cppfront and the new test cleanly using MSVC

And re-remove a few stray `;` that were removed as part of PR #911 and now cause errors because of the pedantic builds

I regenerated `reflect.h` and found two changes that weren't already in the PR, so committing those too

Also including the new test file's run against MSVC showing the five messages mentioned in the PR review, see PR review for more...

* refactor(to_cpp1): pass `0` to `int` parameter instead of `false`

* test: discard unused results

* refactor(to_cpp1): avoid the UFCS macro to workaround compiler bugs

* test: update GCC 13 results

* test: remove Clang 18 results

* refactor(sema): avoid pointer arithmetic

* test: regenerate results

* refactor(sema): avoid pointer arithmetic

* Add regression test result diffs from my machine

MSVC error is resolved

New Clang 12 error in `pure2-last-use.cpp2:938`

* test: comment Clang 12 limitation

* test: apply CI patches

* test: apply CI patches

* test: apply CI patches

* Tweak: Change "final position" to "global token order"

* Minor tweaks

While I'm at it, clean up redundant headers that now we get from cpp2util.h
And it's updating those regression test line-ends...

---------

Signed-off-by: Johel Ernesto Guerrero Peña <[email protected]>
Signed-off-by: Herb Sutter <[email protected]>
Co-authored-by: Herb Sutter <[email protected]>
  • Loading branch information
@JohelEGP @hsutter
JohelEGP and hsutter authored Feb 4, 2024
1 parent 8412611 commit 510eae8
Show file tree
Hide file tree
Showing 56 changed files with 3,874 additions and 413 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
*.exe
source/gen_version.bat
build*/
*.ifc

# Visual Studio cache directory
.vs/
Expand Down
49 changes: 26 additions & 23 deletions include/cpp2util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,7 @@ class out {
#endif
#endif

#define CPP2_UFCS_IDENTITY(...) __VA_ARGS__
#define CPP2_UFCS_REMPARENS(...) __VA_ARGS__

// Ideally, the expression `CPP2_UFCS_IS_NOTHROW` expands to
Expand All @@ -881,18 +882,18 @@ class out {
// we instead make it a template parameter of the UFCS lambda.
// But using a template parameter, Clang also ICEs on an application.
// So we use these `NOTHROW` macros to fall back to the ideal for when not using GCC.
#define CPP2_UFCS_IS_NOTHROW(QUALID,TEMPKW,...) \
#define CPP2_UFCS_IS_NOTHROW(MVFWD,QUALID,TEMPKW,...) \
requires { requires requires { std::declval<Obj>().CPP2_UFCS_REMPARENS QUALID TEMPKW __VA_ARGS__(std::declval<Params>()...); }; \
requires noexcept(std::declval<Obj>().CPP2_UFCS_REMPARENS QUALID TEMPKW __VA_ARGS__(std::declval<Params>()...)); } \
|| requires { requires !requires { std::declval<Obj>().CPP2_UFCS_REMPARENS QUALID TEMPKW __VA_ARGS__(std::declval<Params>()...); }; \
requires noexcept(CPP2_UFCS_REMPARENS QUALID __VA_ARGS__(std::declval<Obj>(), std::declval<Params>()...)); }
#define CPP2_UFCS_IS_NOTHROW_PARAM(...) /*empty*/
#define CPP2_UFCS_IS_NOTHROW_ARG(QUALID,TEMPKW,...) CPP2_UFCS_IS_NOTHROW(QUALID,TEMPKW,__VA_ARGS__)
requires noexcept(MVFWD(CPP2_UFCS_REMPARENS QUALID __VA_ARGS__)(std::declval<Obj>(), std::declval<Params>()...)); }
#define CPP2_UFCS_IS_NOTHROW_PARAM(...) /*empty*/
#define CPP2_UFCS_IS_NOTHROW_ARG(MVFWD,QUALID,TEMPKW,...) CPP2_UFCS_IS_NOTHROW(MVFWD,QUALID,TEMPKW,__VA_ARGS__)
#if defined(__GNUC__) && !defined(__clang__)
#undef CPP2_UFCS_IS_NOTHROW_PARAM
#undef CPP2_UFCS_IS_NOTHROW_ARG
#define CPP2_UFCS_IS_NOTHROW_PARAM(QUALID,TEMPKW,...) , bool IsNothrow = CPP2_UFCS_IS_NOTHROW(QUALID,TEMPKW,__VA_ARGS__)
#define CPP2_UFCS_IS_NOTHROW_ARG(...) IsNothrow
#define CPP2_UFCS_IS_NOTHROW_PARAM(MVFWD,QUALID,TEMPKW,...) , bool IsNothrow = CPP2_UFCS_IS_NOTHROW(MVFWD,QUALID,TEMPKW,__VA_ARGS__)
#define CPP2_UFCS_IS_NOTHROW_ARG(...) IsNothrow
#if __GNUC__ < 11
#undef CPP2_UFCS_IS_NOTHROW_PARAM
#undef CPP2_UFCS_IS_NOTHROW_ARG
Expand All @@ -909,41 +910,43 @@ class out {
// But using a template parameter, Clang also ICEs and GCC rejects a local 'F'.
// Also, Clang rejects the SFINAE test case when using 'std::declval'.
// So we use these `CONSTRAINT` macros to fall back to the ideal for when not using MSVC.
#define CPP2_UFCS_CONSTRAINT_PARAM(...) /*empty*/
#define CPP2_UFCS_CONSTRAINT_ARG(QUALID,TEMPKW,...) \
#define CPP2_UFCS_CONSTRAINT_PARAM(...) /*empty*/
#define CPP2_UFCS_CONSTRAINT_ARG(MVFWD,QUALID,TEMPKW,...) \
requires { CPP2_FORWARD(obj).CPP2_UFCS_REMPARENS QUALID TEMPKW __VA_ARGS__(CPP2_FORWARD(params)...); } \
|| requires { CPP2_UFCS_REMPARENS QUALID __VA_ARGS__(CPP2_FORWARD(obj), CPP2_FORWARD(params)...); }
|| requires { MVFWD(CPP2_UFCS_REMPARENS QUALID __VA_ARGS__)(CPP2_FORWARD(obj), CPP2_FORWARD(params)...); }
#if defined(_MSC_VER)
#undef CPP2_UFCS_CONSTRAINT_PARAM
#undef CPP2_UFCS_CONSTRAINT_ARG
#define CPP2_UFCS_CONSTRAINT_PARAM(QUALID,TEMPKW,...) , bool IsViable = \
#define CPP2_UFCS_CONSTRAINT_PARAM(MVFWD,QUALID,TEMPKW,...) , bool IsViable = \
requires { std::declval<Obj>().CPP2_UFCS_REMPARENS QUALID TEMPKW __VA_ARGS__(std::declval<Params>()...); } \
|| requires { CPP2_UFCS_REMPARENS QUALID __VA_ARGS__(std::declval<Obj>(), std::declval<Params>()...); }
|| requires { MVFWD(CPP2_UFCS_REMPARENS QUALID __VA_ARGS__)(std::declval<Obj>(), std::declval<Params>()...); }
#define CPP2_UFCS_CONSTRAINT_ARG(...) IsViable
#endif

#define CPP2_UFCS_(LAMBDADEFCAPT,QUALID,TEMPKW,...) \
#define CPP2_UFCS_(LAMBDADEFCAPT,MVFWD,QUALID,TEMPKW,...) \
[LAMBDADEFCAPT]< \
typename Obj, typename... Params \
CPP2_UFCS_IS_NOTHROW_PARAM(QUALID,TEMPKW,__VA_ARGS__) \
CPP2_UFCS_CONSTRAINT_PARAM(QUALID,TEMPKW,__VA_ARGS__) \
CPP2_UFCS_IS_NOTHROW_PARAM(MVFWD,QUALID,TEMPKW,__VA_ARGS__) \
CPP2_UFCS_CONSTRAINT_PARAM(MVFWD,QUALID,TEMPKW,__VA_ARGS__) \
> \
CPP2_LAMBDA_NO_DISCARD (Obj&& obj, Params&& ...params) CPP2_FORCE_INLINE_LAMBDA_CLANG \
noexcept(CPP2_UFCS_IS_NOTHROW_ARG(QUALID,TEMPKW,__VA_ARGS__)) CPP2_FORCE_INLINE_LAMBDA -> decltype(auto) \
requires CPP2_UFCS_CONSTRAINT_ARG(QUALID,TEMPKW,__VA_ARGS__) { \
noexcept(CPP2_UFCS_IS_NOTHROW_ARG(MVFWD,QUALID,TEMPKW,__VA_ARGS__)) CPP2_FORCE_INLINE_LAMBDA -> decltype(auto) \
requires CPP2_UFCS_CONSTRAINT_ARG(MVFWD,QUALID,TEMPKW,__VA_ARGS__) { \
if constexpr (requires{ CPP2_FORWARD(obj).CPP2_UFCS_REMPARENS QUALID TEMPKW __VA_ARGS__(CPP2_FORWARD(params)...); }) { \
return CPP2_FORWARD(obj).CPP2_UFCS_REMPARENS QUALID TEMPKW __VA_ARGS__(CPP2_FORWARD(params)...); \
} else { \
return CPP2_UFCS_REMPARENS QUALID __VA_ARGS__(CPP2_FORWARD(obj), CPP2_FORWARD(params)...); \
return MVFWD(CPP2_UFCS_REMPARENS QUALID __VA_ARGS__)(CPP2_FORWARD(obj), CPP2_FORWARD(params)...); \
} \
}

#define CPP2_UFCS(...) CPP2_UFCS_(&,(),,__VA_ARGS__)
#define CPP2_UFCS_TEMPLATE(...) CPP2_UFCS_(&,(),template,__VA_ARGS__)
#define CPP2_UFCS_QUALIFIED_TEMPLATE(QUALID,...) CPP2_UFCS_(&,QUALID,template,__VA_ARGS__)
#define CPP2_UFCS_NONLOCAL(...) CPP2_UFCS_(,(),,__VA_ARGS__)
#define CPP2_UFCS_TEMPLATE_NONLOCAL(...) CPP2_UFCS_(,(),template,__VA_ARGS__)
#define CPP2_UFCS_QUALIFIED_TEMPLATE_NONLOCAL(QUALID,...) CPP2_UFCS_(,QUALID,template,__VA_ARGS__)
#define CPP2_UFCS(...) CPP2_UFCS_(&,CPP2_UFCS_IDENTITY,(),,__VA_ARGS__)
#define CPP2_UFCS_MOVE(...) CPP2_UFCS_(&,std::move,(),,__VA_ARGS__)
#define CPP2_UFCS_FORWARD(...) CPP2_UFCS_(&,CPP2_FORWARD,(),,__VA_ARGS__)
#define CPP2_UFCS_TEMPLATE(...) CPP2_UFCS_(&,CPP2_UFCS_IDENTITY,(),template,__VA_ARGS__)
#define CPP2_UFCS_QUALIFIED_TEMPLATE(QUALID,...) CPP2_UFCS_(&,CPP2_UFCS_IDENTITY,QUALID,template,__VA_ARGS__)
#define CPP2_UFCS_NONLOCAL(...) CPP2_UFCS_(,CPP2_UFCS_IDENTITY,(),,__VA_ARGS__)
#define CPP2_UFCS_TEMPLATE_NONLOCAL(...) CPP2_UFCS_(,CPP2_UFCS_IDENTITY,(),template,__VA_ARGS__)
#define CPP2_UFCS_QUALIFIED_TEMPLATE_NONLOCAL(QUALID,...) CPP2_UFCS_(,CPP2_UFCS_IDENTITY,QUALID,template,__VA_ARGS__)


//-----------------------------------------------------------------------
Expand Down
5 changes: 0 additions & 5 deletions regression-tests/pure2-deducing-pointers-error.cpp2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ main: () -> int = {
pa5 := pppa**;
pa5 = 0; // caught

// TODO: @filipsajdak please take a look
// The bugfix in get_declaration_of(t) to add `&& ri->position() <= t.position()`
// to the condition is correct; it fixes issue #669 by not looking past the first
// declaration of the name in t. However, that change made the following two
// "caught" cases no longer be caught.
fun(a)++; // caught
fp := fun(a);
fp = 0; // caught
Expand Down
1 change: 1 addition & 0 deletions regression-tests/pure2-last-use-error.cpp2
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
issue_890: (x) = { }
Loading

0 comments on commit 510eae8

Please sign in to comment.