[PATCH] D130827: [clang] Fixed a number of typos

Jun Zhang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Jul 31 00:26:03 PDT 2022


junaire added a comment.

Hi, I think the main issue is that your patch contains another patch unintentionally, please fix that :)



================
Comment at: clang/CMakeLists.txt:513
-endif()
-
 option(CLANG_ENABLE_ARCMT "Build ARCMT." ON)
----------------
Why delete these lines?


================
Comment at: clang/include/clang/Basic/JsonSupport.h:109
+                             std::end(ForbiddenChars),
+                             Char) != std::end(ForbiddenChars);
           });
----------------
This is not a typo, right? And I think using `llvm::is_contained` is more impressive?


================
Comment at: clang/include/clang/Sema/Sema.h:4022
+      ExprResult &SrcExpr, bool DoFunctionPointerConversion = false,
+      bool Complain = false, SourceRange OpRangeForComplaining = SourceRange(),
+      QualType DestTypeForComplaining = QualType(),
----------------
Normally we don't clang-format legacy code like this, as it's bad for git blame. 


================
Comment at: clang/lib/Format/Format.cpp:1724
+                        Style->QualifierOrder.end(), "type");
+  if (type == Style->QualifierOrder.end())
     return ParseError::MissingQualifierType;
----------------
ditto


================
Comment at: clang/lib/Headers/opencl-c.h:17856
+intel_sub_group_avc_sic_payload_t __ovld intel_sub_group_avc_sic_configure_ipe(
+    uchar luma_intra_partition_mask, uchar intra_neighbour_availability,
     uchar left_edge_luma_pixels, uchar upper_left_corner_luma_pixel,
----------------
Not sure if we want to format this...


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1880
+      if (I->getOwnKind() != K && I->args_end() !=
+          std::find(I->args_begin(), I->args_end(), Idx)) {
         S.Diag(AL.getLoc(), diag::err_attributes_are_not_compatible) << AL << I;
----------------
ditto


================
Comment at: clang/test/CMakeLists.txt:154
-endif()
-
 # Copy gen_ast_dump_json_test.py to the clang build dir. This allows invoking
----------------
Why drop this? I thought it was intentional?


================
Comment at: clang/test/Interpreter/code-undo.cpp:3
 // RUN:            'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
+// REQUIRES: host-supports-jit
 // UNSUPPORTED: system-aix
----------------
why add this in a typo-fix patch?


================
Comment at: clang/test/Interpreter/execute-weak.cpp:4
 // RUN:            'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
+// REQUIRES: host-supports-jit
 // CHECK-DRIVER: i = 10
----------------
why add this in a typo-fix patch?


================
Comment at: clang/test/Interpreter/execute.cpp:5
 // RUN:            'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
+// REQUIRES: host-supports-jit
 // UNSUPPORTED: system-aix
----------------
why add this in a typo-fix patch?


================
Comment at: clang/test/Interpreter/global-dtor-win.cpp:1
+// clang-format off
+// FIXME: Merge into global-dtor.cpp when exception support arrives on windows-msvc
----------------
Is this in main?


================
Comment at: clang/tools/CMakeLists.txt:17
 add_clang_subdirectory(clang-scan-deps)
-if(HAVE_CLANG_REPL_SUPPORT)
-  add_clang_subdirectory(clang-repl)
----------------
Why drop this?


================
Comment at: clang/tools/clang-repl/ClangRepl.cpp:31
                                               llvm::cl::Hidden);
+static llvm::cl::opt<bool> OptHostSupportsException("host-supports-exception",
+                                                    llvm::cl::Hidden);
----------------
I think this patch contains another patch...


================
Comment at: clang/tools/clang-repl/ClangRepl.cpp:70
 
+// Check if the host environment supports c++ exception handling
+// by querying the existence of symbol __cxa_throw.
----------------
I think this patch contains another patch...


================
Comment at: clang/tools/clang-repl/ClangRepl.cpp:130
 
+  if (OptHostSupportsException) {
+    if (checkExceptionSupport())
----------------
I think this patch contains another patch...


================
Comment at: clang/unittests/CMakeLists.txt:38
 add_subdirectory(CodeGen)
-if(HAVE_CLANG_REPL_SUPPORT)
-  add_subdirectory(Interpreter)
----------------
Why drop this?


================
Comment at: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:116
 
+  // Check if platform does not support exceptions.
+  {
----------------
I think this patch contains another patch...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130827



More information about the cfe-commits mailing list