A perpetual PR for running regression tests #10
Conversation
This file contains hidden or 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
* Typeid cannot be used when -fno-rtti is used. Fixed compilation of generated sources on arm-none-eabi-g++ 13.1. * fully compile time type deduction for GCC and clang * Restored exception message --------- Co-authored-by: Herb Sutter <[email protected]>
And I really wish Git had a reliable way to deal with line-ends when collaborators use both Linux and Windows systems...
* Fix regressions tests * Exclude failing cppfront build on macos-11
Signed-off-by: gregmarr <[email protected]>
(1) Cache values in the initial lookup, which at least prevents duplicate traversals. I think we can do even better here by not doing a linear search at all, but this improvement is dramatic enough in `pure2-last-use.cpp2` test case that I'm going to check it in as at least an interim optimization. (2) Add `stable_vector`. Since `get_declaration_of` can be called before the `vector` symbol table is completely constructed, later pushes into the symbol table could invalidate previously cached iterators (so just doing (1) above by itself would crash). To avoid that problem, I've also added a new stable (and simplified) `stable_vector` that is intended for cppfront's grow-at-the-end uses. Its first use is in the symbol table, where as an unintended side effect it also seems to be faster than the `vector` for just the symbol table's container operations. I wanted to build something like the `stable_vector` type anyway because I plan to also use it (or something similar) as part of getting rid of all the `deque` objects. `deque` is a useful type conceptually, but sadly I didn't realize how terrible `deque` implementations currently often are so I need to rip these out.
Motivated by improving #904 Fixes bugs - initial version wasn't well tested, this one has been validated to run through regression tests with the same internal results as the old unoptimized function A second optimization is currently commented out until I finish regression-testing it too
… case bugs On my machine, with this commit here are some typical cppfront compile time data points: - `reflect.h2`: 0.5s - `pure2-last-use.cpp2`: 0.3s - all 134 regression tests: 6.3s When I added the optimization to cache results of calls to g_d_o and checked the results to make sure they were unchanged, I noticed that in fact g_d_o was occasionally returning different answers in different calls for the same arguments. All the examples I found were corner cases in `pure2-last-use.cpp2`. The lookup bugs I found are now fixed in this commit. This commit moves the `-_debug` command line flag from reporting stack instrumentation to reporting things I'm now coding here: Now using that flag will check the g_d_o results computed with vs. without the two new optimizations and ensure the answers are the same. (Disclaimer: Because this exercise led to a few bug fixes, which were applied to both the old and new functions, this isn't quite identical to comparing it to the true "old" way which contained a couple of I-think-innocuous-but-now-fixed bugs. The proof of the pudding is in the regression tests being identical.)
… investigation Timers now run only in `-_debug` since they slow things down `-_debug` now implies `-verbose` `-verbose` now sorts the timer output in descending cost order
Signed-off-by: Neil Henderson <[email protected]>
`ubuntu-latest` is an alias/pointer which over time changes and points at newer ubuntu images. Different ubuntu images have different compiler versions installed and newer images drop older compiler versions. So to keep the CI builds running successfully it's better to use explicit version requirements. Signed-off-by: Neil Henderson <[email protected]>
And improve timer performance by removing `make_shared`
…f applicable) the standard library to link with (#1080) * Regression test jobs now specify the C++ standard version and (if applicable) the standard library to link with * Add a new job to test clang with libc++ on ubuntu. (All other ubuntu jobs for gcc & clang link with libstdc++.) * Add a new msvc job so that we now test `c++latest` and `c++20`. * Job names now include more information: <short os name> | <compiler> | <c++ std> | <std library> | <full os name> * Reorder jobs so that GCC are first, then clang on ubuntu & macOS, and then msvc on Windows e.g. ubu-24 | g++-14 | c++2b | libstdc++ | ubuntu-24.04 mac-13 | clang++ | c++2b | default | macos-13 win-22 | cl.exe | c++latest | default | windows-2022 * Update msvc test bat files
Build declaration_starts as we go, instead of rebuilding it in pieces later - I may get rid of it entirely later in favor of a better optimization I'm thinking about, but at least now while we have it this is a cleaner way to build it Also make global_token_counter a sema member - part of getting rid of static variables so that compiling more than one file on the same cppfront command line starts fresh each time (there are a few more statics to go)
After a couple of passes of optimizing get_declaration_of to make its traversal of the symbol table more efficient, I decided to rewrite it entirely to build the information during the initial visitation and not scour at the symbol table afterwards to rediscover the information. The code generation for all `*.cpp2` and `*.h2` files in the project is unchanged, so it seems likely that this rewrite is not introducing bugs. If we do find some, they should be easier to fix as the lookup logic is now more direct and built and consumed in just two places. To prevent accidental behavior change, I exhaustively checked every call to get_declaration_of to check whether it returned a different result before and after this change. There were quite a few during debugging which pointed out where I'd overlooked a lookup case, and those are now all fixed. There remained only a few (<10) calls in this entire corpus where g_d_o is now giving a different result, but all are cases where g_d_o was not being consistent previously (repeated calls with the same arguments in the same execution sometimes gave a result and sometimes gave null) and it's now giving the same answer consistently that it gave "sometimes" before. So I think this change is fixing those as (latent) bugs... latent because the compilation output before/after is unchanged, because the previous inconsistency was in cases that didn't actually affect compilation behavior.
Signed-off-by: gregmarr <[email protected]>
…pty check Addresses comment d136153#r142724776
* cpp2::move is now constexpr * Add `inline` too --------- Co-authored-by: Herb Sutter <[email protected]>
* Add `..` syntax to select a member function only 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. * Remove some debug code * Finish removing old g_d_o paths
…whitespace diff) Seems to be just line-ends
This corrects one typo, and rewords some of the parts of the surrounding wording Signed-off-by: ShalokShalom <[email protected]>
…1367) * Add a conditional compilation directive to be compatible with C++26 * fix typo Signed-off-by: vspefs <[email protected]> * Tweak comment before merging --------- Signed-off-by: vspefs <[email protected]> Co-authored-by: Herb Sutter <[email protected]>
See #1373 The programmer can always write `this.` to disambiguate. This originally seemed like a useful constraint, but it creates a tension between generated code and user code. For example, see #1373, and the example foo : @enum type = { s; } which before this commit was rejected because the `@enum` type metafunction generates a function with a parameter named `s`. It seems undesirable to force generated code to uglify its names to avoid conflicts. Also, fix `@enum` for cases like this to type-scope-qualify access to the member.
* Add `is` and `as` support for `std::expected` * Add test-results for `pure2-expected-is-as` test case * Update test-results * [CI] run-tests.sh now allows each compiler config to exclude test files that it doesn't want to run There are 2 macOS test configs which share the same expected results directory (to avoid duplication). This works great except for the new `pure2-expected-is-as` test. This new test code fails to compile (as expected) on both compilers, but produces a slightly different error diagnostic because the path is different. E.g. /Applications/Xcode_14.3.1.app/...etc.../math.h and /Library/Developer/CommandLineTools/...etc.../math.h One option would be to stop sharing the expected results for both of these compilers, but that seems wasteful since it's just one test which fails to compile. So instead the `run-tests` script has a new way to exclude a test from running.
By bringing in @MaxSagebaum's current autodiff code + just rename one unused parameter to _ to keep a clean build at high warning levels Also fix a bug where a nonempty blank line was being considered a violation of -pure-cpp2
Style note: I'm currently trying to follow the pattern of choosing ".get_Y" vs. ".as_Y" names as follows... "get": X::is_Y / X::get_Y pairs - when Y is conceptually "NOT all of" the X - for example, expression_statement::get_expression because even though the expression is most of the statement the expression-statement contains that extra `;` and is at a different conceptual layer "as": X::is_Y / X::as_Y pairs - when Y is conceptually "all of" the X - for example, statement::as_expression_statement because it's really still the whole statement - with the idea that this could legitimately become an actual `as` operator I may not be 100% consistent on that right now, but I think the current API is pretty close to that
For now a string is likely sufficient We can make this a meta::literal reflection type in the future if that's needed
* #1376: Fix crash on non-constructor with 'out this' * #1376: Add regression test. * Minor tidying Removed the eliminated assert Re-ran regression tests --------- Co-authored-by: Herb Sutter <[email protected]>
* Fix bug #1378 and create a regression test. * Renamed new test file to *-error.cpp2 --------- Co-authored-by: Herb Sutter <[email protected]>
* Three more fuzz crashes.
* Add regression test for crash 10.
* Error instead of crash if users write silly things in aliases.
* Fix assertion on comments near end of file.
* Disallow declarations of parents after functions.
* Fix more fuzz crashes.
* Rename error test cases to `*-error.cpp2`
* Minor tidying to fit house style
`exit(1)` -> `exit(EXIT_FAILURE)`
Branch and loop bodies are always enclosed in `{` `}` even when they contain a single line
---------
Co-authored-by: Herb Sutter <[email protected]>
* CI: Replace Clang-18 with CLang-19 * CI: Fix pure Cpp2 regression tests for MSVC
Only generate other operator= functions from (out this, that) if _none_ of the other three were written - generate all of them or none of them Remove the "A2" generation arrow, which also removes a potential second path to a generated (inout this, move that) via both M2 and A2 where M2 was already preferred - this removes the need for a tie-break and embraces the already-preferred path The previous rule did not allow for expressing copy/move-constructible types that are not assignable (rare, but can happen when there are const members or Cpp1 reference members) - that can now be expressed by writing two operator='s, (out this, that) and (out this, move that)
Like copyable, but only construction, not assignment
Thanks to Jeroen Van Antwerpen for reminding me that it is possible to have a polymorphic type that is copyable/movable (in fact, I do that a lot in the Cpp2 reflection API), and so it makes sense for an @interface to have _protected_ copying functions - this way the interface is not copyable by default (correct), but a type that implements the interface has the option of defining copy/move if it wants to Made @interface generate protected copy/move construction/assignment Similarly, removed the restriction that @polymorphic_base types cannot be copyable Made operator= "A1" generation work also for polymorphic types - until now it was restricted to only monomorphic types, but it really should work anytime operator= exists - this permits @interface to just define the general operator= as per usual Cpp2 style and get all four functions No change to the "A3" rule, that tends to work great for non-polymorphic types - but while doing this PR I tried applying it also to polymorphic types and noticed it would frequently cause problems in the presence of polymorphic base types, so leaving A3 alone seems useful and right Made the polymorphic reflection API types @copy_constructible instead of @copyable so as not to require generating assignment (they are current inherently non-assignable because some members are not assignable)
* CI: Fix Windows build command in build workflow * CI: Update regression test results
…tements) Added tests Added sample code that traverses function bodies including if branches That's most of it... the main thing to reflect next is loops, and then a few smaller things like using statements and inspect statements
Also made parameter attributes accessible
Also add expression[_node]::is_assignment_expression for symmetry, even though currently expression-nodes have no other alternatives (they could grow try-expressions in the future) Cleanup: Remove old autodiff stub code which I think is no longer being used / needed for reference - Max please let me know if that's not so
…tialized locals The reason to initialize in declaration order is for things that depend on nested lifetime, which is common for local variables – such as a smart pointer variable followed by another variable whose delayed initialization depends on the smart pointer being constructed and keeping something alive. That doesn’t apply to return values.
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 is a PR pulling hsutter/main into a local branch.
The goal is to never actually merge the PR, but to get notifications when regression tests fail on hsutter/main.