[PATCH] D138016: [Object] Add some more LoongArch support

WÁNG Xuěruì via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 27 03:23:39 PST 2022


xen0n marked 3 inline comments as done.
xen0n added inline comments.


================
Comment at: llvm/lib/Object/ELFObjectFile.cpp:354
+    // D implies F according to LoongArch ISA spec.
+    // It's only two features so don't bother with [[fallthrough]] at present.
+    Features.AddFeature("f");
----------------
MaskRay wrote:
> ` It's only two features so don't bother with [[fallthrough]] at present.` this comment is unnecessary.
The comment was there explaining why code was duplicated instead of the shorter and arguably equally easy-to-understand approach. If it is removed but the duplicated `AddFeature` remained, then different people's coding style preferences could clash and I wanted to avoid such unnecessary debates.

Since you asked, I've thus removed the comment and chose to `[[fallthrough]]` instead. It's only one line after all, and I believe even programmers so used to functional style code must retain some control-flow deciphering skills to navigate in large C++ codebase in the first place...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138016/new/

https://reviews.llvm.org/D138016



More information about the llvm-commits mailing list