DiggerLin marked 6 inline comments as done.
DiggerLin added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:80
+static bool hasExportListLinkerOpts(const ArgStringList &CmdArgs) {
+ for (size_t i = 0; i < CmdArgs.size(); ++i) {
+ llvm::StringRef ArgString(CmdArgs[i]);
----------------
MaskRay wrote:
> https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop
>
> Prefer `!=`
thanks
================
Comment at: clang/test/Driver/aix-ld.c:609
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared (with exp option strings in
other opt).
+// RUN: %clangxx -x c++ -no-canonical-prefixes %s 2>&1 -### \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
----------------
MaskRay wrote:
> If you don't test `clang` in CHECK lines, you can omit
> `-no-canonical-prefixes`
>
> Consider switching to `.cpp` if you always use `-x c++`.
all the test in the aix-ld.c using "-no-canonical-prefixes", in order to keep
the same style. I am prefer to keep as it now. and create a NFC patch to clean
the ""-no-canonical-prefixes" later. And keep the file name as aix-ld.c , it
maybe better to have another NFC to change the file name.
================
Comment at: clang/test/Driver/aix-ld.c:615
+// RUN: -ccc-install-dir
%T/open_xl_aix_install/opt/IBM/openxlC/%open_xl_vrm/bin \
+// RUN: -unwindlib=libunwind \
+// RUN: -Wl,-Z/expall/expfull/a-bE:/a-bexport:/ \
----------------
MaskRay wrote:
> `-u` prefixed long option form is not recommended. `-u` is a short option
> taking a value, therefore an option name typo cannot be detected.
same comment as above, I am prefer for another NFC patch for it. thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119147/new/
https://reviews.llvm.org/D119147
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
