RFC: structure of test/ASTMerge/Inputs

Aleksei Sidorin recently suggested that the layout of test/ASTMerge/Inputs is not very pretty, and I agree. When tests become more complicated than the traditional structure

a.c
Inputs/a1.c
Inputs/a2.c

then it gets harder to tell the inputs for one testcase from the inputs for another. There are a couple of solutions. I list them in order of my decreasing preference:

- a.c-Inputs/*
  This would eliminate the global Inputs directory altogether and have a clearly-named input directory for each individual test.
  It will make it very quick to recognize and navigate to the inputs for a test.
- Inputs/a.c/*
  This would keep inputs in their own directory, but further sequester them into test-specific directories.
  This means you get a cleaner view of the test case files themselves, although especially for ASTMerge they're not terribly useful by themselves.
  It also means you can possibly have files in Inputs/ that are common to multiple tests. Personally I feel this can lead to bloat and interdependencies.
- The status quo, with better naming
  This would keep a flat structure for Inputs/, with filenames sharing a prefix with their test file. With more complicated tests, or if files need to have specific names, this can be problematic.

I'm planning to introduce a diff for this this week, and currently I'm planning to adopt the a.c-Inputs/ scheme. Please let me know if there's something I haven't considered that would argue for a different approach.

Sean

Thank you, Sean.

There is also another option which eliminates Input:

test-name/{main.c,other-files.c}
common/

Example:
exprs-c/{main.c,from.c,to.c}
exprs-cxx/{main.cpp,from.cpp,to.cpp}
ctors/{main.c,from.c,to.c}
common/{stl-like-header.h,system-include.h}
etc.

08.11.2016 00:47, Sean Callanan via cfe-dev пишет:

I like this option even more, because it allows the test's name to be entirely independent from the name of the main file.
Usually, you'll want to have the same name, but I could conceive of situations where the main file might need to be named something special.
I'd put your recommendation as my most favorite, with the caveat that I would generally discourage a "common" directory to avoid interdependencies.

Sean

I'm OK with removing common/. It was only a suggestion.

08.11.2016 04:33, Sean Callanan via cfe-dev пишет:

As promised:

https://reviews.llvm.org/D26571

Sean

I’m leery of the idea of hard coding test names in the lit.cfg file. This is very different from how all other uses of lit in LLVM projects are today, and I think it would be error-prone. It would be very simple for a developer to throw in a new test, not update the lit.cfg file (because you don’t need to anywhere else) and have the test not run.

I have no objection to re-arranging the directories, but we shouldn’t require that lit.cfg list the specific test files. For example you could create subdirectories under the Inputs directory that are named to match the test case, and put the inputs for the test there. This would improve organization without making lit behave differently.

-Chris

To be clear: there would be generic test names (test.c, test.cpp, test.m, test.mm) and the test implementor would just choose one of these. Since each test would be inside its own directory, this would not be a problem.
Do you still have concerns if this doesn’t involve the developer having to change the .cfg file every time?

Sean

What if the author doesn’t name the test source file test.*? Many of our tests don’t follow that convention, and requiring it in one subdirectory seems undesirable to me.

-Chris

Okay. In that case we could do

a/Inputs/a1.c
a/Inputs/a2.c
a/main.c

and we’d still get all the benefits, at the expense of one extra nested directory for inputs. Aleksei, does that look all right to you?

Sean

Updated the diff.

https://reviews.llvm.org/D26571

Sean