CI: Enable pedantic warnings and treat all warnings as errors #911
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
also fix the warnings raised by -pedantic
It's either this or disabling those warnings entirely for MSVC.
|
Thanks! FWIW two flags I use on GCC and Clang that I don't think are included under the other options: |
|
hsutter
added a commit
that referenced
this pull request
Dec 29, 2023
hsutter
added a commit
to JohelEGP/cppfront
that referenced
this pull request
Jan 2, 2024
And re-remove a few stray `;` that were removed as part of PR hsutter#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...
hsutter
added a commit
that referenced
this pull request
Feb 4, 2024
* 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 <johelegp@gmail.com> Signed-off-by: Herb Sutter <herb.sutter@gmail.com> Co-authored-by: Herb Sutter <herb.sutter@gmail.com>
bluetarpmedia
pushed a commit
to bluetarpmedia/cppfront
that referenced
this pull request
Feb 7, 2024
* 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 hsutter#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 <hsutter#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 hsutter#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 <johelegp@gmail.com> Signed-off-by: Herb Sutter <herb.sutter@gmail.com> Co-authored-by: Herb Sutter <herb.sutter@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PRs makes the CI workflows fail if the compiler emits warnings for cppfront code (by using
-Werroron clang/gcc and-WXon MSVC), it also adds-pedanticto clang/gcc to catch additional oversights.Separately, I provide narrowing conversion "fixes", but I am not too fond of this commit, since it feels like the correct thing to do would be to promote
intto whateverstd::ssizereturns in most places, I didn't do that yet as I believe this requires more thought on the design of the compiler's code and want feedback first. The commit could be skipped and done by someone else, or the warning could be disabled (seen that in lots of projects 😛).