add support for riscv64

[futurejones]

Add support for riscv64 as host architecture.

[etcwilde]

It looks like this is just for computing the module triple.
If that's the case, rather than continuing to extend this tower of if statements, would you be okay swapping out the module triple computation for this:

if(NOT SwiftFormat_MODULE_TRIPLE)
  set(module_triple_command "${CMAKE_Swift_COMPILER}" -print-target-info)
  if(CMAKE_Swift_COMPILER_TARGET)
    list(APPEND module_triple_command -target ${CMAKE_Swift_COMPILER_TARGET})
  endif()
  execute_process(COMMAND ${module_triple_command} OUTPUT_VARIABLE target_info_json)
  string(JSON SwiftFormat_MODULE_TRIPLE GET "${target_info_json}" "target" "moduleTriple")
endif()

It scales better and means that this will build for anything that the compiler supports, rather than what we've manually listed out.

Please see the conversation on swiftlang/swift-corelibs-foundation#4959

https://github.com/apple/swift-corelibs-foundation/pull/4959/files#diff-9179d83b9cabd38d5049f3095d2c31d286f406b53443027b1bac119ecc4408c1R38-R54

[futurejones]

@etcwilde, I understand your reasoning for suggesting this change and if this was the only project with this change I would agree.
Unfortunately it is not. This is just one of 19 separate projects in the swift toolchain that require these changes.
https://github.com/orgs/swift-riscv/projects/2
Some of these have already been approved and merged.
For the sake of code conformity across all the different projects I would rather leave this PR as I submitted it if that is acceptable.
Thanks.

[etcwilde]

For the ones that have merged, sure, we can update them later when the next architecture comes along.
I think we've hit a critical mass where expecting identical behavior across all of the repos is unlikely to happen and I would prefer to see incremental improvements over time over not improving anything to keep uniformity.
Sort of like how LLVM rolls out changes to policies, instead of making a gigantic patch that updates all of the code, folks are supposed to clean up the code around what they're working on.

[futurejones]

@etcwilde so you are refusing to accept this PR as is?
If so, please state this clearly so I can take further action.

[etcwilde]

Yes, I have requested changes and given my reasoning.

[futurejones]

@etcwilde thats unfortunate.

[etcwilde]

Sorry, hit the wrong button.

[futurejones]

Closing this PR as requested changes have been unreasonably refused by @etcwilde

[etcwilde]

To be clear, I didn't ask for you to close the PR, I asked for you to change it so that instead of tacking on new architectures to an ever-growing list, we do something that queries the compiler and will work for any architecture that the compiler supports.

[futurejones]

@etcwilde - To be clear.
The purpose of this PR was to "Add support for riscv64 as host architecture."
The code was clean, concise and only impacted swift builds on a riscv64 host.
I reviewed your suggested changes and explained why in the scope of this PR and the Swift on RISCV project I wanted the PR to be accepted without change.
My reasons for this are completely reasonable and justified.
You have stated that you will not accept this PR as is.
So I have closed this PR and moved on.

[ahoppen]

Hey @futurejones,

I think there was a misunderstanding here. We appreciate your effort to port Swift to riscv64 and are happy to take your PR. @etcwilde just noted that there’s a more general solution here that will scale to other architectures in the future. As these sorts of lists grow, it’s important to generalize them because adding to a tower of if-else branches doesn’t scale indefinitely. Where exactly that tipping point is, is obviously subjective but this change seemed as good as any to generalize since you were touching all repositories already.

If I understand correctly, the reason why @etcwilde marked the PR as “request changes” is because he accidentally hit “accept” beforehand and GitHub doesn’t allow changing a review to the “neutral” style, which we usually use. I don’t think it was because he explicitly disapproved your PR, just suggesting improvements.

Going forward, I would suggest that we merge this PR as you proposed it. Cleaning up the if branches with the code snippet that @etcwilde suggested would be an amazing follow-up PR that would be highly appreciated.

[futurejones]

@ahoppen @etcwilde
What's the next step here?