From 79b38b19bb6d4148c8a6fb6b1e64c94ccdc4c185 Mon Sep 17 00:00:00 2001 From: Shaikh Ubaid Date: Sat, 1 Jul 2023 03:08:43 +0530 Subject: [PATCH 1/9] CI: Check debug build with -Werror Also use sccache in place of ccache. sccache seems to be faster --- .github/workflows/CI.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 39eea08e96..3309c61548 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -278,12 +278,14 @@ jobs: - uses: hendrikmuhs/ccache-action@main with: + variant: sccache key: ${{ github.job }}-${{ matrix.os }} - name: Build Linux shell: bash -l {0} run: | ./build0.sh + export CXXFLAGS="-Werror" cmake . -GNinja \ -DCMAKE_BUILD_TYPE=Debug \ -DWITH_LLVM=yes \ @@ -292,8 +294,8 @@ jobs: -DWITH_RUNTIME_STACKTRACE=yes \ -DCMAKE_PREFIX_PATH="$CONDA_PREFIX" \ -DCMAKE_INSTALL_PREFIX=`pwd`/inst \ - -DCMAKE_C_COMPILER_LAUNCHER=ccache \ - -DCMAKE_CXX_COMPILER_LAUNCHER=ccache + -DCMAKE_C_COMPILER_LAUNCHER=sccache \ + -DCMAKE_CXX_COMPILER_LAUNCHER=sccache cmake --build . -j16 --target install From 251297380481ca1a057e535734f059516af2b91f Mon Sep 17 00:00:00 2001 From: Shaikh Ubaid Date: Sat, 1 Jul 2023 03:17:56 +0530 Subject: [PATCH 2/9] CI: Check release build with -Werror --- .github/workflows/CI.yml | 47 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 3309c61548..127153f9e9 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -308,6 +308,53 @@ jobs: ./run_tests.py -b llvm c ./run_tests.py -b llvm c -f + release: + name: Check Release build + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - uses: mamba-org/setup-micromamba@v1 + with: + environment-file: ci/environment.yml + create-args: >- + python=3.10 + bison=3.4 + + - uses: hendrikmuhs/ccache-action@main + with: + variant: sccache + key: ${{ github.job }}-${{ matrix.os }} + + - name: Build Linux + shell: bash -l {0} + run: | + ./build0.sh + export CXXFLAGS="-Werror" + cmake . -GNinja \ + -DCMAKE_BUILD_TYPE=Release \ + -DWITH_LLVM=yes \ + -DLFORTRAN_BUILD_ALL=yes \ + -DWITH_STACKTRACE=no \ + -DWITH_RUNTIME_STACKTRACE=yes \ + -DCMAKE_PREFIX_PATH="$CONDA_PREFIX" \ + -DCMAKE_INSTALL_PREFIX=`pwd`/inst \ + -DCMAKE_C_COMPILER_LAUNCHER=sccache \ + -DCMAKE_CXX_COMPILER_LAUNCHER=sccache + + cmake --build . -j16 --target install + + - name: Test Linux + shell: bash -l {0} + run: | + ctest + ./run_tests.py -s + cd integration_tests + ./run_tests.py -b llvm c + ./run_tests.py -b llvm c -f + cpython_interop: name: Test CPython Interop (@pythoncall) runs-on: ubuntu-latest From e224b02a2ffdfe3a7fa132fd473fd40a17cbdf34 Mon Sep 17 00:00:00 2001 From: Shaikh Ubaid Date: Tue, 11 Jul 2023 00:17:57 +0530 Subject: [PATCH 3/9] Build: Suppress unused warning in parser.tab.cc --- build0.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/build0.sh b/build0.sh index d1f1c279e8..deb7f2da21 100755 --- a/build0.sh +++ b/build0.sh @@ -19,5 +19,11 @@ python src/libasr/wasm_instructions_visitor.py (cd src/lpython/parser && re2c -W -b tokenizer.re -o tokenizer.cpp) (cd src/lpython/parser && bison -Wall -d -r all parser.yy) +python -c "file = 'src/lpython/parser/parser.tab.cc' +with open(file, 'r') as f: text = f.read() +with open(file, 'w') as f: + f.write('[[maybe_unused]] int yynerrs'.join(text.split('int yynerrs'))) +" + grep -n "'" src/lpython/parser/parser.yy && echo "Single quote not allowed" && exit 1 echo "OK" From 74a65be4cc0e49dcafe33af50d97e99ed1dee390 Mon Sep 17 00:00:00 2001 From: Shaikh Ubaid Date: Tue, 11 Jul 2023 00:27:03 +0530 Subject: [PATCH 4/9] LLVM_UTILS: Throw error for unsupported overload_id This fixes the build warnings --- src/libasr/codegen/llvm_utils.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libasr/codegen/llvm_utils.cpp b/src/libasr/codegen/llvm_utils.cpp index 4c2a241a84..b431347435 100644 --- a/src/libasr/codegen/llvm_utils.cpp +++ b/src/libasr/codegen/llvm_utils.cpp @@ -1485,7 +1485,8 @@ namespace LCompilers { break; } default: { - // can exit with error + throw LCompilersException("LLVMUtils::is_ineq_by_value unsupported overload_id " + + std::to_string(overload_id)); } } return builder->CreateCmp(pred, left, right); @@ -1509,7 +1510,8 @@ namespace LCompilers { break; } default: { - // can exit with error + throw LCompilersException("LLVMUtils::is_ineq_by_value unsupported overload_id " + + std::to_string(overload_id)); } } return builder->CreateCmp(pred, left, right); @@ -1556,7 +1558,8 @@ namespace LCompilers { break; } default: { - // can exit with error + throw LCompilersException("LLVMUtils::is_ineq_by_value unsupported overload_id " + + std::to_string(overload_id)); } } cond = builder->CreateAnd(cond, builder->CreateCmp(pred, l, r)); From 43b4a7276409fb841231d19ff0df583cc820a41f Mon Sep 17 00:00:00 2001 From: Shaikh Ubaid Date: Tue, 11 Jul 2023 00:34:43 +0530 Subject: [PATCH 5/9] Fix warnings in release build --- src/lpython/parser/parser.cpp | 2 +- src/lpython/semantics/python_ast_to_asr.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lpython/parser/parser.cpp b/src/lpython/parser/parser.cpp index 381b7c5587..92ab5023a7 100644 --- a/src/lpython/parser/parser.cpp +++ b/src/lpython/parser/parser.cpp @@ -117,7 +117,7 @@ Result parse_python_file(Allocator &al, const std::string &infile, diag::Diagnostics &diagnostics, uint32_t prev_loc, - bool new_parser) { + [[maybe_unused]] bool new_parser) { LPython::AST::ast_t* ast; // We will be using the new parser from now on new_parser = true; diff --git a/src/lpython/semantics/python_ast_to_asr.cpp b/src/lpython/semantics/python_ast_to_asr.cpp index b5e8c153d8..bba948cc08 100644 --- a/src/lpython/semantics/python_ast_to_asr.cpp +++ b/src/lpython/semantics/python_ast_to_asr.cpp @@ -1009,7 +1009,7 @@ class CommonVisitor : public AST::BaseVisitor { ASR::ttype_t* get_type_from_var_annotation(std::string var_annotation, const Location& loc, Vec& dims, - AST::expr_t** m_args=nullptr, size_t n_args=0, + AST::expr_t** m_args=nullptr, [[maybe_unused]] size_t n_args=0, bool raise_error=true, ASR::abiType abi=ASR::abiType::Source, bool is_argument=false) { ASR::ttype_t* type = nullptr; @@ -2416,7 +2416,7 @@ class CommonVisitor : public AST::BaseVisitor { result = left_value >> right_value; break; } - default: { LCOMPILERS_ASSERT(false); } // should never happen + default: { throw SemanticError("ICE: Unknown binary operator", loc); } // should never happen } value = ASR::down_cast(ASR::make_UnsignedIntegerConstant_t( al, loc, result, dest_type)); @@ -2458,7 +2458,7 @@ class CommonVisitor : public AST::BaseVisitor { case (ASR::binopType::Mul): { result = left_value * right_value; break; } case (ASR::binopType::Div): { result = left_value / right_value; break; } case (ASR::binopType::Pow): { result = std::pow(left_value, right_value); break; } - default: { LCOMPILERS_ASSERT(false); } + default: { throw SemanticError("ICE: Unknown binary operator", loc); } } value = ASR::down_cast(ASR::make_RealConstant_t( al, loc, result, dest_type)); @@ -3289,7 +3289,7 @@ class CommonVisitor : public AST::BaseVisitor { void visit_NamedExpr(const AST::NamedExpr_t &x) { this->visit_expr(*x.m_target); ASR::expr_t *target = ASRUtils::EXPR(tmp); - ASR::ttype_t *target_type = ASRUtils::expr_type(target); + [[maybe_unused]] ASR::ttype_t *target_type = ASRUtils::expr_type(target); this->visit_expr(*x.m_value); ASR::expr_t *value = ASRUtils::EXPR(tmp); ASR::ttype_t *value_type = ASRUtils::expr_type(value); From 1e8da8b37165e5a0af6ed8e2273f0ab6a0d3b0e5 Mon Sep 17 00:00:00 2001 From: Shaikh Ubaid Date: Tue, 11 Jul 2023 00:35:04 +0530 Subject: [PATCH 6/9] Commit white space removals --- src/libasr/codegen/llvm_utils.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libasr/codegen/llvm_utils.cpp b/src/libasr/codegen/llvm_utils.cpp index b431347435..2e325ac463 100644 --- a/src/libasr/codegen/llvm_utils.cpp +++ b/src/libasr/codegen/llvm_utils.cpp @@ -4434,21 +4434,21 @@ namespace LCompilers { ASR::ttype_t* int32_type) { /** * Equivalent in C++ - * + * * equality_holds = 1; * inequality_holds = 0; * i = 0; - * + * * while( i < a_len && i < b_len && equality_holds ) { * equality_holds &= (a[i] == b[i]); * inequality_holds |= (a[i] op b[i]); * i++; * } - * + * * if( (i == a_len || i == b_len) && equality_holds ) { * inequality_holds = a_len op b_len; * } - * + * */ llvm::AllocaInst *equality_holds = builder->CreateAlloca( @@ -4673,11 +4673,11 @@ namespace LCompilers { llvm::Module& module, int8_t overload_id) { /** * Equivalent in C++ - * + * * equality_holds = 1; * inequality_holds = 0; * i = 0; - * + * * // owing to compile-time access of indices, * // loop is unrolled into multiple if statements * while( i < a_len && equality_holds ) { @@ -4685,9 +4685,9 @@ namespace LCompilers { * equality_holds &= (a[i] == b[i]); * i++; * } - * + * * return inequality_holds; - * + * */ llvm::AllocaInst *equality_holds = builder->CreateAlloca( From 97b17c661f9d61560a3eecc6ce47c5d48b231449 Mon Sep 17 00:00:00 2001 From: Shaikh Ubaid Date: Tue, 11 Jul 2023 01:09:27 +0530 Subject: [PATCH 7/9] Fix unneeded if --- src/libasr/codegen/llvm_utils.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/libasr/codegen/llvm_utils.cpp b/src/libasr/codegen/llvm_utils.cpp index 2e325ac463..c8f1d89849 100644 --- a/src/libasr/codegen/llvm_utils.cpp +++ b/src/libasr/codegen/llvm_utils.cpp @@ -622,12 +622,7 @@ namespace LCompilers { if( type == nullptr ) { type = get_type_from_ttype_t_util(v_type->m_type, module, arg_m_abi)->getPointerTo(); - } - } - if( type != nullptr ) { - } - if( type != nullptr ) { - break; + } break; } case ASR::array_physical_typeType::FixedSizeArray: { type = llvm::ArrayType::get(get_el_type(v_type->m_type, module), From 96f5287bebb08c51436a56a6554cd87967a9a3b4 Mon Sep 17 00:00:00 2001 From: Shaikh Ubaid Date: Tue, 11 Jul 2023 02:50:53 +0530 Subject: [PATCH 8/9] Fix warnings in lfortran_intrinsics.c --- src/libasr/runtime/lfortran_intrinsics.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libasr/runtime/lfortran_intrinsics.c b/src/libasr/runtime/lfortran_intrinsics.c index 0affc5b056..fb59601e52 100644 --- a/src/libasr/runtime/lfortran_intrinsics.c +++ b/src/libasr/runtime/lfortran_intrinsics.c @@ -1372,7 +1372,7 @@ LFORTRAN_API void _lfortran_read_int32(int32_t *p, int32_t unit_num) if (unit_num == -1) { // Read from stdin FILE *fp = fdopen(0, "r+"); - fread(p, sizeof(int32_t), 1, fp); + (void)fread(p, sizeof(int32_t), 1, fp); fclose(fp); return; } @@ -1380,7 +1380,7 @@ LFORTRAN_API void _lfortran_read_int32(int32_t *p, int32_t unit_num) printf("No file found with given unit\n"); exit(1); } - fread(p, sizeof(int32_t), 1, unit_to_file[unit_num]); + (void)fread(p, sizeof(int32_t), 1, unit_to_file[unit_num]); } LFORTRAN_API void _lfortran_read_char(char **p, int32_t unit_num) @@ -1389,7 +1389,7 @@ LFORTRAN_API void _lfortran_read_char(char **p, int32_t unit_num) // Read from stdin *p = (char*)malloc(16); FILE *fp = fdopen(0, "r+"); - fread(*p, sizeof(char), 16, fp); + (void)fread(*p, sizeof(char), 16, fp); fclose(fp); return; } @@ -1398,7 +1398,7 @@ LFORTRAN_API void _lfortran_read_char(char **p, int32_t unit_num) exit(1); } *p = (char*)malloc(16); - fread(*p, sizeof(char), 16, unit_to_file[unit_num]); + (void)fread(*p, sizeof(char), 16, unit_to_file[unit_num]); } LFORTRAN_API char* _lpython_read(int64_t fd, int64_t n) From ca7b9e1daa583929cfef868b656a233298eb678b Mon Sep 17 00:00:00 2001 From: Shaikh Ubaid Date: Tue, 11 Jul 2023 02:52:26 +0530 Subject: [PATCH 9/9] Fix more warnings --- src/lpython/semantics/python_ast_to_asr.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/lpython/semantics/python_ast_to_asr.cpp b/src/lpython/semantics/python_ast_to_asr.cpp index bba948cc08..b1407e5dc0 100644 --- a/src/lpython/semantics/python_ast_to_asr.cpp +++ b/src/lpython/semantics/python_ast_to_asr.cpp @@ -831,6 +831,8 @@ class CommonVisitor : public AST::BaseVisitor { this->visit_expr(*exprs[i]); ASR::expr_t* expr = nullptr; ASR::call_arg_t arg; + arg.loc.first = -1; + arg.loc.last = -1; if (tmp) { expr = ASRUtils::EXPR(tmp); arg.loc = expr->base.loc; @@ -2362,7 +2364,7 @@ class CommonVisitor : public AST::BaseVisitor { result = left_value >> right_value; break; } - default: { LCOMPILERS_ASSERT(false); } // should never happen + default: { throw SemanticError("ICE: Unknown binary operator", loc); } // should never happen } value = ASR::down_cast(ASR::make_IntegerConstant_t( al, loc, result, dest_type)); @@ -3396,7 +3398,7 @@ class CommonVisitor : public AST::BaseVisitor { ASR::expr_t *left = ASRUtils::EXPR(tmp); this->visit_expr(*x.m_right); ASR::expr_t *right = ASRUtils::EXPR(tmp); - ASR::binopType op; + ASR::binopType op = ASR::binopType::Add /* temporary assignment */; std::string op_name = ""; switch (x.m_op) { case (AST::operatorType::Add) : { op = ASR::binopType::Add; break; } @@ -5602,7 +5604,7 @@ class BodyVisitor : public CommonVisitor { ASR::expr_t *right = ASRUtils::EXPR(tmp); ASR::ttype_t* left_type = ASRUtils::expr_type(left); ASR::ttype_t* right_type = ASRUtils::expr_type(right); - ASR::binopType op; + ASR::binopType op = ASR::binopType::Add /* temporary assignment */; std::string op_name = ""; switch (x.m_op) { case (AST::operatorType::Add) : { op = ASR::binopType::Add; break; } @@ -6313,7 +6315,9 @@ class BodyVisitor : public CommonVisitor { result = (strcmp < 0 || strcmp == 0); break; } - default: LCOMPILERS_ASSERT(false); // should never happen + default: { + throw SemanticError("ICE: Unknown compare operator", x.base.base.loc); // should never happen + } } value = ASR::down_cast(ASR::make_LogicalConstant_t( al, x.base.base.loc, result, type));