remove useless comments and improve useful comments and naming

VKCOM · Nov 6, 2024 · 8124818 · 8124818
1 parent 2d12923
commit 8124818
Show file tree

Hide file tree

Showing 12 changed files with 18 additions and 32 deletions.
diff --git a/compiler/code-gen/vertex-compiler.cpp b/compiler/code-gen/vertex-compiler.cpp
@@ -1803,8 +1803,7 @@ bool try_compile_append_inplace(VertexAdaptor<op_set_dot> root, CodeGenerator &W
   return false;
 }
 
-// This also compiles index of mixed
-void compile_index_of_array(VertexAdaptor<op_index> root, CodeGenerator &W) {
+void compile_index_of_array_or_mixed(VertexAdaptor<op_index> root, CodeGenerator &W) {
   bool used_as_rval = root->rl_type != val_l;
   if (!used_as_rval) {
     kphp_assert(root->has_key());
@@ -1878,7 +1877,7 @@ void compile_index(VertexAdaptor<op_index> root, CodeGenerator &W) {
       W << ShapeGetIndex(root->array(), root->key());
       break;
     default:
-      compile_index_of_array(root, W);
+      compile_index_of_array_or_mixed(root, W);
   }
 }
 

diff --git a/compiler/compiler.cpp b/compiler/compiler.cpp
@@ -267,7 +267,7 @@ bool compiler_execute(CompilerSettings *settings) {
     >> PipeC<CheckAbstractFunctionDefaults>{}
     >> PipeC<CalcEmptyFunctions>{}
     >> PassC<CalcActualCallsEdgesPass>{}
-    >> SyncC<FilterOnlyActuallyUsedFunctionsF>{} // removes unused func calls
+    >> SyncC<FilterOnlyActuallyUsedFunctionsF>{}
     >> PassC<RemoveEmptyFunctionCallsPass>{}
     >> PassC<PreprocessBreakPass>{}
     >> PassC<ConvertSprintfCallsPass>{}
@@ -281,9 +281,9 @@ bool compiler_execute(CompilerSettings *settings) {
     >> PipeC<CFGBeginF>{}
     >> SyncC<CFGBeginSync>{}
     >> PassC<CloneStrangeConstParams>{}
-    >> PassC<CollectMainEdgesPass>{} // collects edges for type inference
+    >> PassC<CollectMainEdgesPass>{}
     >> SyncC<TypeInfererF>{}
-    >> SyncC<CheckRestrictionsF>{} // type inferer finishes here
+    >> SyncC<CheckRestrictionsF>{}
     >> PipeC<CFGEndF>{}
     >> PassC<CheckClassesPass>{}
     >> PassC<CheckConversionsPass>{}

diff --git a/compiler/inferring/primitive-type.h b/compiler/inferring/primitive-type.h
@@ -1,12 +1,11 @@
 // Compiler for PHP (aka KPHP)
-// Copyright (c) 2020 LLC «V Kontakte»
+// Copyright (c) 2024 LLC «V Kontakte»
 // Distributed under the GPL v3 License, see LICENSE.notice.txt
 
 #pragma once
 
-#include <string>
-
-// The order is important!
+// Order is important. These values are compared with operator< 
+// in some passes (in type inferring, for example)
 enum PrimitiveType {
   tp_any,
   tp_Null,

diff --git a/compiler/inferring/type-data.cpp b/compiler/inferring/type-data.cpp
@@ -361,9 +361,6 @@ const TypeData *TypeData::const_read_at(const Key &key) const {
       ClassPtr aa = G->get_class("ArrayAccess");
       assert(aa && "Cannot find ArrayAccess");
 
-      // Why offsetSet for ArrayAccess is going through here
-      // And in some later pass (FinalCheckPass, for example) check that access via [.] is enabled only for arrays and classes that implements ArrayAccess (by
-      // chain)
       if (aa->is_parent_of(klass)) {
         return get_type(tp_mixed);
       }
@@ -408,7 +405,6 @@ const TypeData *TypeData::const_read_at(const MultiKey &multi_key) const {
 }
 
 void TypeData::make_structured() {
-  // TODO fix here for writing into objects that implements ArrayAccess
   // 'lvalue $s[idx]' makes $s array-typed: strings and tuples keep their types only for read-only operations
   if (ptype() < tp_array) {
     PrimitiveType new_ptype = type_lca(ptype(), tp_array);
@@ -542,7 +538,7 @@ void TypeData::set_lca_at(const MultiKey &multi_key, const TypeData *rhs, bool s
 
   for (const Key &key : multi_key) {
     auto *prev = cur;
-    cur = cur->write_at(key); // HERE
+    cur = cur->write_at(key);
     // handle writing to a subkey of mixed (when cur is not structured)
     if (cur == nullptr) {
       if (prev->ptype() == tp_mixed) {

diff --git a/compiler/inferring/type-inferer.cpp b/compiler/inferring/type-inferer.cpp
@@ -25,10 +25,10 @@ void TypeInferer::recalc_node(Node *node) {
 }
 
 void TypeInferer::add_node(Node *node) {
+  // fprintf (stderr, "tinf::add_node %d %p %s\n", get_thread_id(), node, node->get_description().c_str());
   if (!node->was_recalc_started_at_least_once()) {
     recalc_node(node);
   }
-  // fprintf (stderr, "tinf::add_node %d %p %s\n", get_thread_id(), node, node->get_description().c_str());
 }
 
 void TypeInferer::add_edge(const Edge *edge) {
@@ -98,8 +98,7 @@ std::vector<Task *> TypeInferer::get_tasks() {
 void TypeInferer::do_run_queue() {
   NodeQueue &q = Q.get();
 
-  // in bad case first node add new VarNode into the Q queue
-  while (!q.empty()) { // when add here 2 nodes (when bad case)
+  while (!q.empty()) {
     Node *node = q.front();
 
     node->start_recalc();

diff --git a/compiler/inferring/var-node.cpp b/compiler/inferring/var-node.cpp
@@ -11,7 +11,6 @@
 #include "compiler/inferring/restriction-match-phpdoc.h"
 #include "compiler/vertex.h"
 
-// Check in add_node that adding f$ArrayAccess$offsetGet
 class VarNodeRecalc : public NodeRecalc {
   static void on_restricted_type_mismatch(const tinf::Edge *edge, const TypeData *type_restriction);
 

diff --git a/compiler/make/make.cpp b/compiler/make/make.cpp
@@ -395,13 +395,9 @@ static std::vector<File *> create_obj_files(MakeSetup *make, Index &obj_dir, con
     for (File *f : deps) {
       vk::hash_combine(hash, vk::std_hash(f->name));
     }
-
-
     auto intermediate_file_name = fmt_format("{}_{:x}.{}", name_and_files.first, hash,
                                              G->settings().dynamic_incremental_linkage.get() ? "so" : "o");
     File *obj_file = obj_dir.insert_file(std::move(intermediate_file_name));
-
-
     make->create_objs2obj_target(std::move(deps), obj_file);
     objs.push_back(obj_file);
   }
@@ -448,7 +444,7 @@ static std::string get_light_runtime_compiler_options() {
   for (vk::string_view option : options) {
     for (vk::string_view prohibit_substr : black_list_substrings) {
       if (vk::contains(option, prohibit_substr)) continue;
-      s << option << " ";
+      s   tion << " ";
     }
   }
   s << "-std=c++20 ";

diff --git a/compiler/make/objs-to-obj-target.h b/compiler/make/objs-to-obj-target.h
@@ -15,8 +15,7 @@ class Objs2ObjTarget : public Target {
     std::stringstream ss;
     ss << settings->cxx.get() <<
        " " << settings->cxx_toolchain_option.get() <<
-       " " << 
-       settings->incremental_linker_flags.get() <<
+       " " << settings->incremental_linker_flags.get() <<
        " -o " << target() <<
        " " << dep_list();
     return ss.str();

diff --git a/compiler/pipes/check-classes.cpp b/compiler/pipes/check-classes.cpp
@@ -43,7 +43,7 @@ inline void CheckClassesPass::analyze_class(ClassPtr klass) {
   if (klass->does_need_codegen()) {
     check_instance_fields_inited(klass);
   }
-  if (klass->can_be_php_autoloaded && !klass->is_builtin() && !klass->need_generated_stub && !klass->internal_interface) { // TODO think about it later
+  if (klass->can_be_php_autoloaded && !klass->is_builtin() && !klass->need_generated_stub) {
     kphp_error(klass->file_id->main_function->body_seq == FunctionData::body_value::empty,
                fmt_format("class {} can be autoloaded, but its file contains some logic (maybe, require_once files with global vars?)\n",
                           klass->as_human_readable()));

diff --git a/compiler/pipes/check-ub.cpp b/compiler/pipes/check-ub.cpp
@@ -206,9 +206,6 @@ void fix_ub_dfs(VertexPtr v, UBMergeData *data, VertexPtr parent = VertexPtr())
     stage::set_location(save_location);
 
     if (res > 0) {
-      // TODO
-      // Find out how fix ub pass works
-      // and get rid of useless warning
       bool supported = vk::any_of_equal(v->type(), op_set, op_set_value, op_push_back, op_push_back_return, op_array, op_index)
                        || OpInfo::rl(v->type()) == rl_set;
       if (supported) {

diff --git a/runtime-common/core/array_access.h b/runtime-common/core/array_access.h
@@ -5,7 +5,9 @@
 #pragma once
 #include "runtime-common/core/class-instance/refcountable-php-classes.h"
 
-// TODO may be add guard to force include only from runtime-core.h?
+#ifndef INCLUDED_FROM_KPHP_CORE
+  #error "this file must be included only from runtime-core.h"
+#endif
 
 struct C$ArrayAccess : public may_be_mixed_base {
   virtual int get_hash() const noexcept = 0;

diff --git a/tests/cpp/runtime/_runtime-tests-env.cpp b/tests/cpp/runtime/_runtime-tests-env.cpp
@@ -101,4 +101,4 @@ void f$ArrayAccess$$offsetSet(class_instance<C$ArrayAccess> const & /*v$this*/,
 
 void f$ArrayAccess$$offsetUnset(class_instance<C$ArrayAccess> const & /*v$this*/, mixed const & /*v$offset*/) noexcept {
   assert(0 && "this code shouldn't be executed and only for linkage test");
-}
+}