Skip to content

[llvm] add tool to verify mustache library #111487

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

Closed
wants to merge 79 commits into from

Conversation

PeterChou1
Copy link
Contributor

Depends on #105893

Adds a tool to verify the moustache spec

Copy link

github-actions bot commented Apr 3, 2025

✅ With the latest revision this PR passed the Python code formatter.

@PeterChou1
Copy link
Contributor Author

Please call this llvm-mustache-spec -- let's try to avoid polluting the global namespace with niche tools.

I also wonder whether this shouldn't be a util rather than tool -- this is purely for internal use, right?

Yes this a utility purely for internal use, I did not realize there was already a utils folder

@PeterChou1 PeterChou1 requested review from nikic and ilovepi April 3, 2025 20:04
@ilovepi
Copy link
Contributor

ilovepi commented Apr 3, 2025

ah, right. I forgot we build things under utils that's a much better place for this.

Overall the patch is much better. Once it's moved to utils I think it will be good. @nikic I seem to recall that we now prefer markdown over .rst files. Is that correct? I tried to find documentation for the policy, but I seem to be missing it in my grep/googling.

@nikic
Copy link
Contributor

nikic commented Apr 4, 2025

Overall the patch is much better. Once it's moved to utils I think it will be good. @nikic I seem to recall that we now prefer markdown over .rst files. Is that correct? I tried to find documentation for the policy, but I seem to be missing it in my grep/googling.

I believe doc files that are used for man page generation are required to use .rst, because we don't want ability to generate man pages with just a sphinx dependency.

@@ -0,0 +1,12 @@
llvm-mustachespec - LLVM tool to test Mustache Compliance Library
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
llvm-mustachespec - LLVM tool to test Mustache Compliance Library
llvm-mustachespec - LLVM tool to test Mustache Library Compliance

Possibly?

llvm-mustachespec - LLVM tool to test Mustache Compliance Library
=================================================================

llvm-mustachespec test the mustache spec conformance of the LLVM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
llvm-mustachespec test the mustache spec conformance of the LLVM
llvm-mustachespec tests the mustache spec conformance of the LLVM

=================================================================

llvm-mustachespec test the mustache spec conformance of the LLVM
mustache library. The spec can be found here https://github.com/mustache/spec
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mustache library. The spec can be found here https://github.com/mustache/spec
mustache library. The spec can be found here: `Mustache Spec <https://github.com/mustache/spec>`_.

or something.


.. program:: llvm-mustachespec

Outputs the number of tests failures and success in the spec
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Outputs the number of tests failures and success in the spec
Outputs the number of tests failures and successes in the spec.

@@ -0,0 +1,104 @@
//===- llvm-mustachespec.cpp - The LLVM Modular Optimizer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//===- llvm-mustachespec.cpp - The LLVM Modular Optimizer

We no longer include this line.

// Triple Mustache - Standalone
// Triple Mustache With Padding
// Standalone Indentation
// Implicit Iterator - Triple mustache
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether it would make sense to include this list directly in the program, and report these as "xfail"? That way it's easy to see whether there are any unexpected failures without manually comparing to this list.

cl::OneOrMore);

void runThroughTest(StringRef InputFile) {
llvm::outs() << "Running Tests: " << InputFile << "\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
llvm::outs() << "Running Tests: " << InputFile << "\n";
outs() << "Running Tests: " << InputFile << "\n";

You're using using namespace llvm, don't need all these llvm:: prefixes.

Comment on lines +51 to +53
if (auto EC = BufferOrError.getError()) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the error get reported?

return;
}
// Get test
Array *Obj = (*Json).getAsObject()->getArray("tests");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Array *Obj = (*Json).getAsObject()->getArray("tests");
Array *Obj = Json->getAsObject()->getArray("tests");

This looks a bit odd, does this work?

if (Partials) {
for (auto &PartialPairs : *Partials->getAsObject()) {
const auto &[Partial, Str] = PartialPairs;
T.registerPartial((*Str.getAsString()).str(), Partial.str());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
T.registerPartial((*Str.getAsString()).str(), Partial.str());
T.registerPartial(Str.getAsString()->str(), Partial.str());

ilovepi added a commit to ilovepi/llvm-project that referenced this pull request Jun 1, 2025
This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in llvm#111487.

Co-authored-by: Peter Chou <[email protected]>
ilovepi added a commit that referenced this pull request Jun 4, 2025
This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in #111487.

Co-authored-by: Peter Chou <[email protected]>
ilovepi added a commit that referenced this pull request Jun 5, 2025
This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in #111487.

Co-authored-by: Peter Chou <[email protected]>
ilovepi added a commit that referenced this pull request Jun 5, 2025
This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in #111487.

Co-authored-by: Peter Chou <[email protected]>
ilovepi added a commit that referenced this pull request Jun 7, 2025
This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in #111487.

Co-authored-by: Peter Chou <[email protected]>
ilovepi added a commit that referenced this pull request Jun 7, 2025
This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in #111487.

Co-authored-by: Peter Chou <[email protected]>
ilovepi added a commit that referenced this pull request Jun 11, 2025
This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in #111487.

Co-authored-by: Peter Chou <[email protected]>
ilovepi added a commit that referenced this pull request Jun 11, 2025
This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in #111487.

Co-authored-by: Peter Chou <[email protected]>
ilovepi added a commit that referenced this pull request Jun 11, 2025
#142813)

This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in #111487.

Co-authored-by: Peter Chou <[email protected]>
@ilovepi
Copy link
Contributor

ilovepi commented Jun 11, 2025

This was completed in #142813

@ilovepi ilovepi closed this Jun 11, 2025
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
llvm#142813)

This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in llvm#111487.

Co-authored-by: Peter Chou <[email protected]>
akuhlens pushed a commit to akuhlens/llvm-project that referenced this pull request Jun 24, 2025
llvm#142813)

This is a cli tool to that tests the conformance of LLVM's mustache
implementation against the public Mustache spec, hosted at
https://github.com/mustache/spec. This is a revised version of the
patches in llvm#111487.

Co-authored-by: Peter Chou <[email protected]>
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

Successfully merging this pull request may close these issues.

3 participants