problem_format: add C++ templates #158

Riolku · 2024-01-08T05:02:02Z

Many templates have been floating around in the DMOJ community for validation and input handling in checkers. This commit aims to consolidate them. It has two main goals:

Correct. Duh.
Simple. Other templates that circulate, including the ones I have published, are too complex. People naively try and write their own. I am sick and tired of reading over incorrect validators.

These templates forgo some principles of good design (such as object-oriented programming) in favour of pure simplicity. They should be simple enough that they are understandable by the broader community, and are not a black box. Hopefully this also dissuades re-writing.

quantum5 · 2024-01-08T05:14:08Z

Can we make sure the templates pass clang-format?

Riolku · 2024-01-08T05:15:34Z

Yes, I'd be happy to add CI for that. Would you also like me to add some tests for the methods using ctest?

quantum5 · 2024-01-08T05:17:03Z

Would you also like me to add some tests for the methods using ctest?

Sure, but at least make sure they build or something.

docs/problem_format/cpp_psetting_templates.md

kiritofeng

Otherwise LGTM

docs/problem_format/cpp_psetting_templates.md

sample_files/problem_setting/test/testsuite/esoteric_validator/cases/noeol/input

kiritofeng

Mostly LGTM

kiritofeng · 2024-09-10T21:22:08Z

sample_files/problem_setting/examples/identical_checker.cpp

+  return parsedInt;
+}
+template <typename T>
+std::vector<T> readIntArray(size_t N, long long lo, long long hi) {


Should probably add as a comment that this will need modification if you want to read unsigned long long.

Added to docs. I think fixing this function to handle uint64 is a pain... The best option that comes to mind is having readInt have a template parameter for the return type. Inferring template parameters from the arguments, like doing readInt(T lo, T hi) doesn't work because the compiler shits itself if you try readInt(0, 1e5) or readInt(1, 4U) because the types are different.

Hard-requiring uint64 is rare, so I think this limitation is ok, since the alternatives sacrifice ergonomics greatly.

Don't you need to specify a template type anyway since T doesn't appear in the argument list at all? So the problem you are trying to address with type inference doesn't actually exist...

Yes, readIntArray forces specifying the template parameter and this is intentional - it's impossible to ergonomically infer it from the arguments, and having the return value always be long longs is not great, unlike with readInt where immediate downcasting is possible.

Then, just declare lo and hi as T to avoid pointlessly adding limitations.

For reasons discussed before this isn't possible. Specifically, readIntArray(N, 1, 1e9) no longer works in that case.

readIntArray(N, 1, 1e9) never works because you always need to specify a return type due to the template being on the return value only. So you are optimizing for a case that doesn't exist.

If anything, using T in the parameter list will allow readIntArray(N, 1, 100); to work.

Right, so to make readIntArray(N, 1, 1e9) work, we force passing the template parameter. Then readIntArray<int>(N, 1, 1e9) works.

The proper way to do this in C++ if you care about type deduction would be something like:

template<typename T> T readInteger(T lo, T hi) { // ... } constexpr auto readInt = &readInteger<int>; // more aliases as needed template<typename T> std::vector<T> readIntArray(size_t N, T lo, T hi) { // ... }

You know what, this is always going to be terrible and I am done trying to police this into something sane.

kiritofeng

LGTM

quantum5 · 2024-12-23T05:03:33Z

sample_files/problem_setting/examples/identical_checker.cpp

+  return parsedInt;
+}
+template <typename T>
+std::vector<T> readIntArray(size_t N, long long lo, long long hi) {


Don't you need to specify a template type anyway since T doesn't appear in the argument list at all? So the problem you are trying to address with type inference doesn't actually exist...

sample_files/problem_setting/examples/identical_checker.cpp

quantum5 · 2024-12-23T05:04:37Z

sample_files/problem_setting/examples/identical_checker.cpp

+namespace CheckerCodes {
+int AC = 0;
+int WA = 1;
+int PE = 2;
+int PARTIAL = 7;
+} // namespace CheckerCodes


Sounds like this should be an enum.

If we use a bare enum, then AC, WA, PE, PARTIAL leak into the symbol table, and if we use an enum class, then std::exit(CheckerCodes::AC) doesn't work

If you don't want the enum to leak into the global scope, then declare it in a namespace. Don't declare a bunch of random mutable global variables. You can actually do CheckerCodes::AC = 1337; like this.

I feel like it's best to just use constexpr and be done with it, no?

I dont see what you mean with CheckerCodes::AC = 1337, TMK that requires a previous declaration of the variable(?)

Changed to constexpr

sample_files/problem_setting/examples/identical_checker.cpp

quantum5 · 2024-12-23T05:05:27Z

sample_files/problem_setting/examples/standard_interactor.cpp

+  int c = getchar();
+  ConsumeResult result = NO_WHITESPACE;
+  while (isspace(c) && c != EOF) {
+    if (result == NO_WHITESPACE) {
+      result = NO_LINES;
+    }
+    if (c == '\r' || c == '\n') {
+      result = LINES;
+    }
+    c = getchar();
+  }


Can we write this without the repeating c = getchar()?

Not sure how. Would probably require adding break and continue or something, which is in my opinion worse.

sample_files/problem_setting/test/run_test.sh

quantum5 · 2024-12-23T06:18:52Z

sample_files/problem_setting/examples/identical_checker.cpp

+namespace CheckerCodes {
+int AC = 0;
+int WA = 1;
+int PE = 2;
+int PARTIAL = 7;
+} // namespace CheckerCodes


If you don't want the enum to leak into the global scope, then declare it in a namespace. Don't declare a bunch of random mutable global variables. You can actually do CheckerCodes::AC = 1337; like this.

quantum5 · 2024-12-23T06:19:43Z

sample_files/problem_setting/examples/identical_checker.cpp

+  return parsedInt;
+}
+template <typename T>
+std::vector<T> readIntArray(size_t N, long long lo, long long hi) {


Then, just declare lo and hi as T to avoid pointlessly adding limitations.

sample_files/problem_setting/examples/identical_checker.cpp

sample_files/problem_setting/test/run_test.sh

sample_files/problem_setting/examples/identical_checker.cpp

quantum5 · 2024-12-23T21:04:43Z

sample_files/problem_setting/examples/identical_checker.cpp

+  return parsedInt;
+}
+template <typename T>
+std::vector<T> readIntArray(size_t N, long long lo, long long hi) {


readIntArray(N, 1, 1e9) never works because you always need to specify a return type due to the template being on the return value only. So you are optimizing for a case that doesn't exist.

If anything, using T in the parameter list will allow readIntArray(N, 1, 100); to work.

sample_files/problem_setting/examples/identical_checker.cpp

quantum5 · 2024-12-23T21:07:07Z

sample_files/problem_setting/identical_checker_interactor.cpp

+#include <cstdlib>
+#include <fstream>
+#include <iostream>
+#include <regex.h>


Actually wait... why are you using the POSIX regex library instead of the C++11 one?

The C++11 library takes 5 seconds to compile on DMOJ, which is unacceptably slow for a checker/interactor. The C library compiles instantly, since it's available as a shared library instead of a template header library.

Okay fine. I don't like the POSIX library, but C++ sucks.

Many templates have been floating around in the DMOJ community for validation and input handling in checkers. This commit aims to consolidate them. It has two main goals: - Correct. Duh. - Simple. Other templates that circulate, including the ones I have published, are too complex. People naively try and write their own. I am sick and tired of reading over incorrect validators. These templates forgo some principles of good design (such as object-oriented programming) in favour of pure simplicity. They should be simple enough that they are understandable by the broader community, and are not a black box. Hopefully this also dissuades re-writing.

quantum5

I don't want to deal with this anymore.

quantum5 · 2024-12-24T08:19:27Z

sample_files/problem_setting/examples/identical_checker.cpp

+  return parsedInt;
+}
+template <typename T>
+std::vector<T> readIntArray(size_t N, long long lo, long long hi) {


The proper way to do this in C++ if you care about type deduction would be something like:

template<typename T> T readInteger(T lo, T hi) { // ... } constexpr auto readInt = &readInteger<int>; // more aliases as needed template<typename T> std::vector<T> readIntArray(size_t N, T lo, T hi) { // ... }

You know what, this is always going to be terrible and I am done trying to police this into something sane.

quantum5 · 2024-12-24T08:31:55Z

sample_files/problem_setting/identical_checker_interactor.cpp

+#include <cstdlib>
+#include <fstream>
+#include <iostream>
+#include <regex.h>


Okay fine. I don't like the POSIX library, but C++ sucks.

kiritofeng reviewed Jan 8, 2024

View reviewed changes

docs/problem_format/cpp_psetting_templates.md Outdated Show resolved Hide resolved

docs/problem_format/cpp_psetting_templates.md Outdated Show resolved Hide resolved

Riolku force-pushed the cpp-templates branch 2 times, most recently from 03b9a4b to 76ec663 Compare January 9, 2024 16:01

Riolku force-pushed the cpp-templates branch from 76ec663 to 8a83444 Compare June 13, 2024 22:08

Riolku force-pushed the cpp-templates branch 10 times, most recently from 24cee83 to efffd68 Compare September 7, 2024 21:20

kiritofeng requested a review from Xyene September 8, 2024 14:56

kiritofeng reviewed Sep 8, 2024

View reviewed changes

docs/problem_format/cpp_psetting_templates.md Outdated Show resolved Hide resolved

sample_files/problem_setting/test/testsuite/esoteric_validator/cases/noeol/input Show resolved Hide resolved

Riolku force-pushed the cpp-templates branch 3 times, most recently from 62f93d3 to 195ee98 Compare September 9, 2024 01:16

kiritofeng reviewed Sep 10, 2024

View reviewed changes

kiritofeng approved these changes Sep 11, 2024

View reviewed changes

quantum5 requested changes Dec 23, 2024

View reviewed changes

Riolku force-pushed the cpp-templates branch from 195ee98 to ef14302 Compare December 23, 2024 05:43

quantum5 requested changes Dec 23, 2024

View reviewed changes

Riolku force-pushed the cpp-templates branch 2 times, most recently from 160629b to 8b9ac02 Compare December 23, 2024 19:08

quantum5 requested changes Dec 23, 2024

View reviewed changes

Riolku force-pushed the cpp-templates branch from 8b9ac02 to f796f19 Compare December 24, 2024 05:50

quantum5 approved these changes Dec 24, 2024

View reviewed changes

kiritofeng merged commit 549d173 into DMOJ:master Dec 24, 2024
2 checks passed

problem_format: add C++ templates #158

problem_format: add C++ templates #158

Uh oh!

Conversation

Riolku commented Jan 8, 2024

Uh oh!

quantum5 commented Jan 8, 2024

Uh oh!

Riolku commented Jan 8, 2024

Uh oh!

quantum5 commented Jan 8, 2024

Uh oh!

Uh oh!

Uh oh!

kiritofeng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kiritofeng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kiritofeng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment