task-create-implicit-pass #19

Closed
Dudarenko wants to merge 0 commits from task-create-implicit-pass into master
Collaborator
No description provided.
Dudarenko self-assigned this 2023-12-12 06:05:24 +00:00
Alexander_KS was assigned by Dudarenko 2023-12-12 06:05:24 +00:00
Dudarenko added 3 commits 2023-12-12 06:05:24 +00:00
Alexander_KS approved these changes 2023-12-12 08:25:49 +00:00
Alexander_KS left a comment
Owner

я бы начал смотреть комментарии снизу вверх - от самой нижней функции и выше.

я бы начал смотреть комментарии снизу вверх - от самой нижней функции и выше.
@@ -205,6 +208,8 @@ set(TRANSFORMS
${TR_GV}
${TR_PRIV_DEL}
${TR_CONV})
Owner

ошибка при слиянии, cmake не строится

ошибка при слиянии, cmake не строится
@@ -2057,2 +2058,4 @@
else if (curr_regime == FIX_COMMON_BLOCKS)
fixCommonBlocks(allFuncInfo, commonBlocks, &project);
else if (curr_regime == SET_IMPLICIT_NONE)
ImplicitCheck(&project);
Owner

в проход передавать проект в целом не разрешается, поэтому нужно перевести этот проход на проход по файлам, так как ему не зачем что то выполнять над проектом.

в проход передавать проект в целом не разрешается, поэтому нужно перевести этот проход на проход по файлам, так как ему не зачем что то выполнять над проектом.
@@ -0,0 +14,4 @@
using std::string;
using std::to_string;
static map<char, SgType*> implicit;
Owner

про глобальные переменные комментарий ниже

про глобальные переменные комментарий ниже
@@ -0,0 +18,4 @@
static set<SgSymbol*> vars;
static vector<SgSymbol*> toDecl;
static void InsertToMap(SgType* type, SgExpression* letters) {
Owner

тут происходит что то странное, надо бы это по нормальному сделать. А почему берется второй символ (letter[1]), а не первый? Как размер строки, которая выдается через sunparse, можно покрыть все случаи?

тут происходит что то странное, надо бы это по нормальному сделать. А почему берется второй символ (letter[1]), а не первый? Как размер строки, которая выдается через sunparse, можно покрыть все случаи?
@@ -0,0 +40,4 @@
break;
default :
//printf("IMPLICIT bad format (can bew only {letter : letter} or {letter})");
throw;
Owner

через printInernal, см комментарии ниже

через printInernal, см комментарии ниже
@@ -0,0 +48,4 @@
}
static void InsertDefaultToMap() {
Owner

тут наверно лишняя пустая строка между intType и realType и не нужные { }

тут наверно лишняя пустая строка между intType и realType и не нужные { }
@@ -0,0 +65,4 @@
static void CheckVariables(SgStatement* function) {
for (int k = 0; k < function->numberOfChildrenList1(); k++) {
SgStatement* state = function->childList1(k);
Owner

выход из этого цикла можно сделать как только встретился исполняемый оператор? еще если не сделать выход

if (state->variant() == CONTAINS_STMT)
    break;

то будет анализироваться блок описания вложенных функций. Аналогично и в основной функции - если мы не выйдем по этому варианту, то будет проблема с вложенными функциями

выход из этого цикла можно сделать как только встретился исполняемый оператор? еще если не сделать выход ``` if (state->variant() == CONTAINS_STMT) break; ``` то будет анализироваться блок описания вложенных функций. Аналогично и в основной функции - если мы не выйдем по этому варианту, то будет проблема с вложенными функциями
@@ -0,0 +70,4 @@
if (state->expr(0) && (state->expr(0)->variant() == VAR_REF || state->expr(0)->variant() == ARRAY_REF)) {
SgSymbol* symbol = state->expr(0)->symbol();
auto x = declaratedInStmt(symbol, NULL, false);
Owner

переменная Х как то не говорит о том, что это такое

переменная Х как то не говорит о том, что это такое
@@ -0,0 +79,4 @@
if (implicit.find(firstLetter) != implicit.end()) {
if (implicit[firstLetter]->variant() != symbol->type()->variant()) {
symbol->setType(implicit[firstLetter]);
Owner

вот так просто поменять тип? главное, чтобы он был правильный, то что ниже делается через new - не правильно. Нужно либо через copyPtr или просто сохранять тот тип, что там выдается.

вот так просто поменять тип? главное, чтобы он был правильный, то что ниже делается через new - не правильно. Нужно либо через copyPtr или просто сохранять тот тип, что там выдается.
@@ -0,0 +81,4 @@
if (implicit[firstLetter]->variant() != symbol->type()->variant()) {
symbol->setType(implicit[firstLetter]);
if (symbol->declaredInStmt() == NULL) {
Owner

а зачем еще раз проверяет, что он не объявлен, если это делается выше?

а зачем еще раз проверяет, что он не объявлен, если это делается выше?
@@ -0,0 +85,4 @@
toDecl.push_back(symbol);
}
}
else {
Owner

скобки лишние, и у if выше тоже

скобки лишние, и у if выше тоже
@@ -0,0 +93,4 @@
//printf("Variable - ");
//printf(symbol->identifier());
//printf(" - not found in IMPLICIT\n");
throw;
Owner

вместо этого нужно использовать printInternalError, примеры есть почти во всех файлах проекта. Вообще везде нужно добавлять проверки и хотя бы это печатать в printInternalError, если поведение отличается от ожидаемого, чтобы потом не искать, где упало.

вместо этого нужно использовать printInternalError, примеры есть почти во всех файлах проекта. Вообще везде нужно добавлять проверки и хотя бы это печатать в printInternalError, если поведение отличается от ожидаемого, чтобы потом не искать, где упало.
@@ -0,0 +99,4 @@
}
}
makeDeclaration(toDecl, function, NULL);
Owner

третий параметр не обязателен

третий параметр не обязателен
@@ -0,0 +111,4 @@
for (int i = 0; i < project->numberOfFiles(); i++) {
SgFile file = project->file(i);
int coutOfFunctions;
Owner

лучше эту переменную все же заполнить, или удалить

лучше эту переменную все же заполнить, или удалить
@@ -0,0 +115,4 @@
for (int j = 0; j < file.numberOfFunctions(); j++) {
SgStatement* function = file.functions(j);
implicit.clear();
Owner

не нужно использовать глобальные переменные без надобности

не нужно использовать глобальные переменные без надобности
@@ -0,0 +121,4 @@
//fill implicit
for (int k = 0; k < function->numberOfChildrenList1(); k++) {
Owner

не лучший вариант прохода по всем операторам функции, так как скорее всего функция взятия конкретного оператора будет проходит по всем оператора функции с начала и до этого номера, что не эффективно. Лучше это делать так, как показано в инструкции - начиная с первого и перемещаясь через lexNext.

не лучший вариант прохода по всем операторам функции, так как скорее всего функция взятия конкретного оператора будет проходит по всем оператора функции с начала и до этого номера, что не эффективно. Лучше это делать так, как показано в инструкции - начиная с первого и перемещаясь через lexNext.
@@ -0,0 +128,4 @@
if (state->variant() == IMPL_DECL) {
if (state->expr(0) == NULL) {
Owner

зачем пустой блок? лучше тогда != NULL

зачем пустой блок? лучше тогда != NULL
@@ -0,0 +140,4 @@
string what_type = one->lhs()->sunparse();
if (what_type.find("kind=") != std::string::npos) {
Owner

что здесь имеется в виду? kind можно получить и без анпарсинга, так как помимо этого ключевого слова, есть или и другой способ задания. Подробности можно посмотреть в LoopAnalyzer/loop_analyzer.cpp:2567 в функции getSizeOfType.

что здесь имеется в виду? kind можно получить и без анпарсинга, так как помимо этого ключевого слова, есть или и другой способ задания. Подробности можно посмотреть в LoopAnalyzer/loop_analyzer.cpp:2567 в функции getSizeOfType.
@@ -0,0 +142,4 @@
if (what_type.find("kind=") != std::string::npos) {
SgType* baseType = new SgType(one->lhs()->type()->variant());
Owner

зачем создавать новый тип? его нельзя разве использовать? это тут явно лишнее

зачем создавать новый тип? его нельзя разве использовать? это тут явно лишнее
@@ -0,0 +152,4 @@
}
else {
SgType* type = new SgType(one->lhs()->type()->variant());
Owner

аналогично, зачем создавать новый тип? вообще наверно можно было бы его скопировать, так как это явно правильнее, чем создавать по варианту, так как тут не учтен еще kind/len.

аналогично, зачем создавать новый тип? вообще наверно можно было бы его скопировать, так как это явно правильнее, чем создавать по варианту, так как тут не учтен еще kind/len.
@@ -0,0 +154,4 @@
SgType* type = new SgType(one->lhs()->type()->variant());
SgExpression* letters = one->lhs()->operand(1);
Owner

думаю, что можно было бы использовать просто lhs() вместо operand.

думаю, что можно было бы использовать просто lhs() вместо operand.
@@ -0,0 +164,4 @@
}
break;
Owner

оператор IMPLICIT может быть только один в каждой функции?

оператор IMPLICIT может быть только один в каждой функции?
@@ -0,0 +167,4 @@
break;
}
if (state->variant() == CONTROL_END) {
Owner

ошибочное условие, что здесь имелось в виду? CONTROL_END является оператором закрытия любого блока, в том числе и блока описания интерфейсов. Нам достаточно убедиться, что начались исполняемые операторы или выполнялась проверка на isSgExecutableStmt(state).

ошибочное условие, что здесь имелось в виду? CONTROL_END является оператором закрытия любого блока, в том числе и блока описания интерфейсов. Нам достаточно убедиться, что начались исполняемые операторы или выполнялась проверка на isSgExecutableStmt(state).
@@ -0,0 +171,4 @@
SgStatement* state = new SgStatement(IMPL_DECL);
auto cp = function->childList1(0)->controlParent();
function->childList1(0)->insertStmtBefore(*state, *cp);
Owner

родитель оператора для вставки есть сама функция, поэтому в данном случае получать ср нет смысла, так как это равно function. Логично бы было использовать insertAfter для вставки нового оператора первым. Вопрос - всегда ли implicit должен быть первым ?

родитель оператора для вставки есть сама функция, поэтому в данном случае получать ср нет смысла, так как это равно function. Логично бы было использовать insertAfter для вставки нового оператора первым. Вопрос - всегда ли implicit должен быть первым ?
@@ -0,0 +1,7 @@
#pragma once
#include "Utils/SgUtils.h"
#include "string"
Owner

все стандартные инклуды делаются через <string>, а не в кавычках

все стандартные инклуды делаются через `<string>`, а не в кавычках
@@ -0,0 +4,4 @@
#include "string"
#include "map"
void ImplicitCheck(SgProject* project);
Owner

соответственно тут прототип должен быть SgFile*

соответственно тут прототип должен быть SgFile*
Alexander_KS closed this pull request 2024-02-19 06:11:33 +00:00

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Alexander_KS/SAPFOR#19