[flang-dev] (Strawman) Costed plan for LLVM-ification of F18

[Timothy Keith via flang-dev]

My comments and questions are below in line.

Tim

> Hi list
>
> As discussed on previous threads, we need to propose a plan for making F18 more LLVM-like in style and use of LLVM APIs and facilities. David is working on getting the permissions to put this into a github project in LLVM so we can collaborate . In order to make progress ahead of that happening, I'd like to have a go on email.
>
> For each area I would like to capture a list of work items we'll commit to doing before merge to monorepo. I want us to be happy we can achieve these in a short time period so we can propose a new merging date. I'd also like to create a list of items we'll commit to fixing after merging to the monorepo and a timeline for getting that done.
>
> Note that I'll only cover API and coding style work items in this thread. The other initiatives we can discuss in other threads. I'll just say that we'll propose cmake changes and porting test suite to lit in the pre-merge stuff and setting up build bots in the post-merge stuff.
>
> When he has access, David can create two projects one for pre-merge and one for post-merge and we'll put everything in there.
>
> Please consider the below a strawman. All dates can change, all list items can change position, new ones can be added, items can be deleted, etc.
>
> What do people think about these lists? Do we think those two dates are do-able?
>
> Ta
> Rich
>
> Proposal:
> We will make the following style changes before merging to the monorepo
>
> F18 changes to make it more LLVM-like in code style
> 1. Rationalise headers to put public headers in /include and not /lib
> 2. Unify F18's clang-format file to match LLVMs

We should discuss what should be in .clang-format with an eye to make it more like LLVM's.

> 3. Rename all .cc files to .cpp
> 4. Switch module names to starting with capital letters

What does "module" mean in this context?

> Increase use of LLVM APIs and utilities in F18
> a. Switch F18 custom File handling to LLVM's File handling (helps with non-POSIX platform support)
> b. Change uses of <iostream> to LLVM's streams

F18 doesn't  use <iostream>.

> c. Migrate use of std::list to a suitable alternative in LLVM's API

I would say: replace std::list with a better data structure where appropriate.

> d. Use llvm_unreachable with an error message for unreachable cases

What does llvm_unreachable do for us?

> e. Use llvm::Error instead of error codes if and when error codes are used.

I don't know of any use of error codes.

> We would like to aim for a merge date of Monday 24th Feb to merge to the monorepo with all of the above changes committed to F18 master.
>
> We then propose to make the following changes after merging to the monorepo.
>
> F18 changes to make it more LLVM-like in code style
> We will commit to a perform a one-off exercise where we automatically audit the code to find these instances and bring them in line.
> 1. Eliminate braces from all single-line if statements

This seems like a really bad idea and would like to see a reason for it.

> 2. Eliminate all uses of else-after-return

Doing this blindly is also a bad idea.

> 3. Add doxygen infrastructure so we can generate docs
>
> Increase use of LLVM APIs and utilities in F18
> a. std::string → StringRef where appropriate

These aren't similar data structures. std::string owns the string data and StringRef does not. std::string_view is now a standard class for the latter case.

> b. std::vector → llvm::SmallVector where appropriate
> c. std::set → llvm::SmallSet/llvm::StringSet/llvm::DenseSet where appropriate
> d. std::map → llvm::StringMap/llvm::DenseMap where appropriate

As long as "where appropriate" means there is tangible benefit to changing from a standard type to a non-standard one. Not just change for the sake of change.

> e. Audit functions in include/flang/common and port to LLVM equivalents (e.g. builtin_popcount)
>
> Assuming we hit the above merge date, we think we can commit to making these changes before the LLVM11 branch is taken in June.
>
> After that date, we will continue to make progress towards LLVM style and API usage by fixing things as we find them during development and enforce the new style through code review.
>
> A few specific areas that have been mentioned before that we will tackle in this way are:
>    - Add doxygen style comments and file comments
>    - Find more expressive names for classes, files, etc.

Names are already supposed to be expressive. Improvements are always welcome.

>    - Refactor code to use early exits

Where it improves the code, not blindly. As with improving names, there is no reason to wait for any specific date to make improvements to the code.

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/flang-dev/attachments/20200129/122c4b5f/attachment.html>