Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibWeb: Remove clang-format FIXME in CryptoAlgorithms when TRY_OR_THROW_OOM wrapped around variant.visit is happier #23580

Open
ADKaster opened this issue Mar 14, 2024 · 1 comment

Comments

@ADKaster
Copy link
Member

Observe the following test case:

andrew@Andrews-MacBook-Pro:~/Source/serenity$ clang-format --version
clang-format version 17.0.6
andrew@Andrews-MacBook-Pro:~/Source/serenity$ clang-format -i test.cpp 
andrew@Andrews-MacBook-Pro:~/Source/serenity$ cat test.cpp
auto maybe_error = handle.visit(
    [&](::Crypto::PK::RSAPublicKey<> const& public_key) -> ErrorOr<void> {
        jwk.n = TRY(base64_url_uint_encode(public_key.modulus()));
        jwk.e = TRY(base64_url_uint_encode(public_key.public_exponent()));
        return {};
    },
    [&](::Crypto::PK::RSAPrivateKey<> const& private_key) -> ErrorOr<void> {
        jwk.n = TRY(base64_url_uint_encode(private_key.modulus()));
        jwk.e = TRY(base64_url_uint_encode(private_key.public_exponent()));

        // 11. If the [[type]] internal slot of key is "private":
        //    1. Set the attributes named d, p, q, dp, dq, and qi of jwk according to the corresponding definitions in JSON Web Algorithms [JWA], Section 6.3.2.
        jwk.d = TRY(base64_url_uint_encode(private_key.private_exponent()));
        // FIXME: Add p, q, dq, qi

        // 12. If the underlying RSA private key represented by the [[handle]] internal slot of key is represented by more than two primes,
        //     set the attribute named oth of jwk according to the corresponding definition in JSON Web Algorithms [JWA], Section 6.3.2.7
        // FIXME: We don't support more than 2 primes on RSA keys
        return {};
    },
    [](auto) -> ErrorOr<void> {
        VERIFY_NOT_REACHED();
    });
andrew@Andrews-MacBook-Pro:~/Source/serenity$ vim test.cpp
andrew@Andrews-MacBook-Pro:~/Source/serenity$ cat test.cpp
TRY_OR_THROW_OOM(vm, handle.visit(
    [&](::Crypto::PK::RSAPublicKey<> const& public_key) -> ErrorOr<void> {
        jwk.n = TRY(base64_url_uint_encode(public_key.modulus()));
        jwk.e = TRY(base64_url_uint_encode(public_key.public_exponent()));
        return {};
    },
    [&](::Crypto::PK::RSAPrivateKey<> const& private_key) -> ErrorOr<void> {
        jwk.n = TRY(base64_url_uint_encode(private_key.modulus()));
        jwk.e = TRY(base64_url_uint_encode(private_key.public_exponent()));

        // 11. If the [[type]] internal slot of key is "private":
        //    1. Set the attributes named d, p, q, dp, dq, and qi of jwk according to the corresponding definitions in JSON Web Algorithms [JWA], Section 6.3.2.
        jwk.d = TRY(base64_url_uint_encode(private_key.private_exponent()));
        // FIXME: Add p, q, dq, qi

        // 12. If the underlying RSA private key represented by the [[handle]] internal slot of key is represented by more than two primes,
        //     set the attribute named oth of jwk according to the corresponding definition in JSON Web Algorithms [JWA], Section 6.3.2.7
        // FIXME: We don't support more than 2 primes on RSA keys
        return {};
    },
    [](auto) -> ErrorOr<void> {
        VERIFY_NOT_REACHED();
    }));
andrew@Andrews-MacBook-Pro:~/Source/serenity$ clang-format test.cpp 
TRY_OR_THROW_OOM(vm, handle.visit([&](::Crypto::PK::RSAPublicKey<> const& public_key) -> ErrorOr<void> {
        jwk.n = TRY(base64_url_uint_encode(public_key.modulus()));
        jwk.e = TRY(base64_url_uint_encode(public_key.public_exponent()));
        return {}; }, [&](::Crypto::PK::RSAPrivateKey<> const& private_key) -> ErrorOr<void> {
        jwk.n = TRY(base64_url_uint_encode(private_key.modulus()));
        jwk.e = TRY(base64_url_uint_encode(private_key.public_exponent()));

        // 11. If the [[type]] internal slot of key is "private":
        //    1. Set the attributes named d, p, q, dp, dq, and qi of jwk according to the corresponding definitions in JSON Web Algorithms [JWA], Section 6.3.2.
        jwk.d = TRY(base64_url_uint_encode(private_key.private_exponent()));
        // FIXME: Add p, q, dq, qi

        // 12. If the underlying RSA private key represented by the [[handle]] internal slot of key is represented by more than two primes,
        //     set the attribute named oth of jwk according to the corresponding definition in JSON Web Algorithms [JWA], Section 6.3.2.7
        // FIXME: We don't support more than 2 primes on RSA keys
        return {}; }, [](auto) -> ErrorOr<void> { VERIFY_NOT_REACHED(); }));
andrew@Andrews-MacBook-Pro:~/Source/serenity$ 

With the extra macro wrapped around the variant visit, clang-format murders the nicely stacked lambdas.

cc @rymiel , do you know if there's any upstream issues that this is a dup of?

@rymiel
Copy link

rymiel commented Mar 14, 2024

I reduced the config that causes this down to {AlignOperands: DontAlign, ColumnLimit: 0}, which is odd, because according to documentation, AlignOperands should only apply to operators and ternaries, and neither are present. If ColumnLimit: 0 is removed, changing AlignOperands between Align and DontAlign changes nothing, which is what I'd expect.
I couldn't find any existing issues for this.
I'll try to investigate if AlignOperands might be doing something odd here (but it's likely I can't spend much time on it and won't find anything)

EDIT: the operator here is apparently the comma, this doesn't happen if you remove the vm parameter. Still strange, though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants