Skip to content

Commit

Permalink
[clang-tidy] Add readability braces around statements (#13730)
Browse files Browse the repository at this point in the history
Enable config "readability-braces-around-statements" [1]. Also, commit
d045742 had config syntax error that unintentionally disabled tidy checks.
Auto fix up the files after fixing that mistake.

[1] https://releases.llvm.org/11.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-braces-around-statements.html
  • Loading branch information
dgkimura authored and my-ship-it committed Nov 1, 2024
1 parent fb7a3ae commit e53b57c
Show file tree
Hide file tree
Showing 64 changed files with 401 additions and 130 deletions.
3 changes: 2 additions & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ Checks: |
modernize-use-nullptr,
modernize-use-default-member-init,
modernize-use-bool-literals,
modernize-use-using
modernize-use-using,
readability-braces-around-statements,
readability-make-member-function-const,
readability-non-const-parameter,
readability-static-accessed-through-instance,
Expand Down
4 changes: 4 additions & 0 deletions src/backend/gpopt/CGPOptimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,19 @@ CGPOptimizer::GPOPTOptimizedPlan(
errmsg(
"GPORCA failed to produce a plan, falling back to planner");
if (serialized_error_msg)
{
errdetail("%s", serialized_error_msg);
}
errfinish(ex.Filename(), ex.Line(), nullptr);
}
}

*had_unexpected_failure = gpopt_context.m_is_unexpected_failure;

if (serialized_error_msg)
{
pfree(serialized_error_msg);
}
}
GPOS_CATCH_END;
return plStmt;
Expand Down
4 changes: 4 additions & 0 deletions src/backend/gpopt/gpdbwrappers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1658,7 +1658,9 @@ gpdb::GpdbEreportImpl(int xerrcode, int severitylevel, const char *xerrmsg,
errcode(xerrcode);
errmsg("%s", xerrmsg);
if (xerrhint)
{
errhint("%s", xerrhint);
}
errfinish(filename, lineno, funcname);
}
}
Expand Down Expand Up @@ -2642,7 +2644,9 @@ gpdb::MDCacheNeedsReset(void)
mdcache_invalidation_counter_registered = true;
}
if (last_mdcache_invalidation_counter == mdcache_invalidation_counter)
{
return false;
}
else
{
last_mdcache_invalidation_counter = mdcache_invalidation_counter;
Expand Down
2 changes: 2 additions & 0 deletions src/backend/gpopt/translate/CContextDXLToPlStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,10 @@ CContextDXLToPlStmt::GetStaticPruneResult(ULONG scanId)
// GPDB_12_MERGE_FIXME: we haven't seen the scan id yet, this scan id is likely for dynamic pruning.
// When we can, remove this check
if ((scanId - 1) >= m_static_prune_results.size())
{
GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiDXL2PlStmtConversion,
GPOS_WSZ_LIT("dynamic pruning"));
}
return m_static_prune_results[scanId - 1];
}
void
Expand Down
4 changes: 4 additions & 0 deletions src/backend/gpopt/translate/CQueryMutators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -837,11 +837,15 @@ CQueryMutators::MakeVarInDerivedTable(Node *node,
const ULONG attno = gpdb::ListLength(context->m_lower_table_tlist) + 1;
TargetEntry *tle = nullptr;
if (IsA(node, Aggref) || IsA(node, GroupingFunc))
{
tle = GetTargetEntryForAggExpr(context->m_mp, context->m_mda, node,
attno);
}
else if (IsA(node, Var))
{
tle = gpdb::MakeTargetEntry((Expr *) node, (AttrNumber) attno, nullptr,
false);
}

context->m_lower_table_tlist =
gpdb::LAppend(context->m_lower_table_tlist, tle);
Expand Down
56 changes: 56 additions & 0 deletions src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2960,19 +2960,33 @@ CTranslatorDXLToPlStmt::TranslateDXLWindow(
win_frame_leading_dxlnode->GetOperator())
->ParseDXLFrameBoundary();
if (lead_boundary_type == EdxlfbUnboundedPreceding)
{
window->frameOptions |= FRAMEOPTION_END_UNBOUNDED_PRECEDING;
}
if (lead_boundary_type == EdxlfbBoundedPreceding)
{
window->frameOptions |= FRAMEOPTION_END_OFFSET_PRECEDING;
}
if (lead_boundary_type == EdxlfbCurrentRow)
{
window->frameOptions |= FRAMEOPTION_END_CURRENT_ROW;
}
if (lead_boundary_type == EdxlfbBoundedFollowing)
{
window->frameOptions |= FRAMEOPTION_END_OFFSET_FOLLOWING;
}
if (lead_boundary_type == EdxlfbUnboundedFollowing)
{
window->frameOptions |= FRAMEOPTION_END_UNBOUNDED_FOLLOWING;
}
if (lead_boundary_type == EdxlfbDelayedBoundedPreceding)
{
window->frameOptions |= FRAMEOPTION_END_OFFSET_PRECEDING;
}
if (lead_boundary_type == EdxlfbDelayedBoundedFollowing)
{
window->frameOptions |= FRAMEOPTION_END_OFFSET_FOLLOWING;
}
if (0 != win_frame_leading_dxlnode->Arity())
{
window->endOffset =
Expand All @@ -2988,19 +3002,33 @@ CTranslatorDXLToPlStmt::TranslateDXLWindow(
win_frame_trailing_dxlnode->GetOperator())
->ParseDXLFrameBoundary();
if (trail_boundary_type == EdxlfbUnboundedPreceding)
{
window->frameOptions |= FRAMEOPTION_START_UNBOUNDED_PRECEDING;
}
if (trail_boundary_type == EdxlfbBoundedPreceding)
{
window->frameOptions |= FRAMEOPTION_START_OFFSET_PRECEDING;
}
if (trail_boundary_type == EdxlfbCurrentRow)
{
window->frameOptions |= FRAMEOPTION_START_CURRENT_ROW;
}
if (trail_boundary_type == EdxlfbBoundedFollowing)
{
window->frameOptions |= FRAMEOPTION_START_OFFSET_FOLLOWING;
}
if (trail_boundary_type == EdxlfbUnboundedFollowing)
{
window->frameOptions |= FRAMEOPTION_START_UNBOUNDED_FOLLOWING;
}
if (trail_boundary_type == EdxlfbDelayedBoundedPreceding)
{
window->frameOptions |= FRAMEOPTION_START_OFFSET_PRECEDING;
}
if (trail_boundary_type == EdxlfbDelayedBoundedFollowing)
{
window->frameOptions |= FRAMEOPTION_START_OFFSET_FOLLOWING;
}
if (0 != win_frame_trailing_dxlnode->Arity())
{
window->startOffset =
Expand All @@ -3012,7 +3040,9 @@ CTranslatorDXLToPlStmt::TranslateDXLWindow(
child_contexts->Release();
}
else
{
window->frameOptions = FRAMEOPTION_DEFAULTS;
}
}

SetParamIds(plan);
Expand Down Expand Up @@ -3217,7 +3247,9 @@ ContainsSetReturningFuncOrOp(const CDXLNode *project_list_dxlnode,
{
case EdxlopScalarFuncExpr:
if (CDXLScalarFuncExpr::Cast(op)->ReturnsSet())
{
return true;
}
break;
case EdxlopScalarOpExpr:
{
Expand All @@ -3226,7 +3258,9 @@ ContainsSetReturningFuncOrOp(const CDXLNode *project_list_dxlnode,
const IMDFunction *md_func =
md_accessor->RetrieveFunc(md_sclar_op->FuncMdId());
if (md_func->ReturnsSet())
{
return true;
}
break;
}
default:
Expand All @@ -3250,16 +3284,24 @@ SanityCheckProjectSetTargetList(List *targetlist)
(IsA(expr, OpExpr) && ((OpExpr *) expr)->opretset))
{
if (IsA(expr, FuncExpr))
{
args = ((FuncExpr *) expr)->args;
}
else
{
args = ((OpExpr *) expr)->args;
}
if (gpdb::ExpressionReturnsSet((Node *) args))
{
return false;
}
continue;
}

if (gpdb::ExpressionReturnsSet((Node *) expr))
{
return false;
}
}
return true;
}
Expand All @@ -3274,9 +3316,11 @@ CTranslatorDXLToPlStmt::TranslateDXLProjectSet(
// GPDB_12_MERGE_FIXME: had we generated a DXLProjectSet in ORCA we wouldn't
// have needed to be defensive here...
if ((*result_dxlnode)[EdxlresultIndexFilter]->Arity() > 0)
{
GPOS_RAISE(
gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
GPOS_WSZ_LIT("Unsupported one-time filter in ProjectSet node"));
}

// create project set (nee result) plan node
ProjectSet *project_set = MakeNode(ProjectSet);
Expand Down Expand Up @@ -3330,9 +3374,11 @@ CTranslatorDXLToPlStmt::TranslateDXLProjectSet(
// double check the targetlist is kosher
// we are only doing this because ORCA didn't do it...
if (!SanityCheckProjectSetTargetList(plan->targetlist))
{
GPOS_RAISE(
gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
GPOS_WSZ_LIT("Unexpected target list entries in ProjectSet node"));
}

return (Plan *) project_set;
}
Expand All @@ -3356,8 +3402,10 @@ CTranslatorDXLToPlStmt::TranslateDXLResult(
// actual result node
if (ContainsSetReturningFuncOrOp((*result_dxlnode)[EdxlresultIndexProjList],
m_md_accessor))
{
return TranslateDXLProjectSet(result_dxlnode, output_context,
ctxt_translation_prev_siblings);
}

// create result plan node
Result *result = MakeNode(Result);
Expand Down Expand Up @@ -3420,9 +3468,11 @@ CTranslatorDXLToPlStmt::TranslateDXLResult(
// double check the targetlist is kosher
// we are only doing this because ORCA didn't do it...
if (!SanityCheckProjectSetTargetList(plan->targetlist))
{
GPOS_RAISE(
gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
GPOS_WSZ_LIT("Unexpected target list entries in ProjectSet node"));
}

return (Plan *) result;
}
Expand Down Expand Up @@ -4063,9 +4113,11 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(
// partition Oid in the child's target list, but we don't use it for
// anything in GPDB.
if (m_cmd_type == CMD_UPDATE)
{
(void) AddJunkTargetEntryForColId(&dml_target_list, &child_context,
phy_dml_dxlop->ActionColId(),
"DMLAction");
}

if (m_cmd_type == CMD_UPDATE || m_cmd_type == CMD_DELETE)
{
Expand All @@ -4076,8 +4128,10 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(
"gp_segment_id");
}
if (m_cmd_type == CMD_UPDATE && phy_dml_dxlop->IsOidsPreserved())
{
AddJunkTargetEntryForColId(&dml_target_list, &child_context,
phy_dml_dxlop->GetTupleOid(), "oid");
}

// Add a Result node on top of the child plan, to coerce the target
// list to match the exact physical layout of the target table,
Expand Down Expand Up @@ -4126,7 +4180,9 @@ CTranslatorDXLToPlStmt::TranslateDXLDml(

// ORCA plans all updates as split updates
if (m_cmd_type == CMD_UPDATE)
{
dml->splitUpdate = true;
}

plan->lefttree = child_plan;
plan->righttree = NULL;
Expand Down
2 changes: 2 additions & 0 deletions src/backend/gpopt/translate/CTranslatorDXLToScalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -733,8 +733,10 @@ CTranslatorDXLToScalar::TranslateDXLScalarWindowRefToScalar(
window_func->winagg = dxlop->IsSimpleAgg();

if (dxlop->GetDxlWinStage() != EdxlwinstageImmediate)
{
GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLError,
GPOS_WSZ_LIT("Unsupported window function stage"));
}

// translate the arguments of the window function
window_func->args = TranslateScalarChildren(window_func->args,
Expand Down
20 changes: 20 additions & 0 deletions src/backend/gpopt/translate/CTranslatorQueryToDXL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,13 @@ CTranslatorQueryToDXL::CTranslatorQueryToDXL(
// If this is a subquery, make a copy of the parent's mapping, otherwise
// initialize a new, empty, mapping.
if (var_colid_mapping)
{
m_var_to_colid_map = var_colid_mapping->CopyMapColId(m_mp);
}
else
{
m_var_to_colid_map = GPOS_NEW(m_mp) CMappingVarColId(m_mp);
}

m_query_level_to_cte_map = GPOS_NEW(m_mp) HMUlCTEListEntry(m_mp);
m_dxl_cte_producers = GPOS_NEW(m_mp) CDXLNodeArray(m_mp);
Expand Down Expand Up @@ -269,7 +273,9 @@ CTranslatorQueryToDXL::~CTranslatorQueryToDXL()
CRefCount::SafeRelease(m_dxl_query_output_cols);

if (m_query_level == 0)
{
GPOS_DELETE(m_context);
}
}

//---------------------------------------------------------------------------
Expand Down Expand Up @@ -524,13 +530,17 @@ CTranslatorQueryToDXL::TranslateSelectQueryToDXL()

// RETURNING is not supported yet.
if (m_query->returningList)
{
GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
GPOS_WSZ_LIT("RETURNING clause"));
}

// ON CONFLICT is not supported yet.
if (m_query->onConflict)
{
GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
GPOS_WSZ_LIT("ON CONFLICT clause"));
}

if (m_query->limitOption == LIMIT_OPTION_WITH_TIES)
GPOS_RAISE(gpdxl::ExmaDXL, gpdxl::ExmiQuery2DXLUnsupportedFeature,
Expand Down Expand Up @@ -3343,9 +3353,13 @@ CTranslatorQueryToDXL::NoteDistributionPolicyOpclasses(const RangeTblEntry *rte)
gpdb::GetDefaultDistributionOpclassForType(typeoid);

if (opclasses[i] == default_opclass)
{
contains_default_hashops = true;
}
else
{
contains_nondefault_hashops = true;
}
}
}

Expand All @@ -3362,21 +3376,25 @@ CTranslatorQueryToDXL::NoteDistributionPolicyOpclasses(const RangeTblEntry *rte)
{
if (m_context->m_distribution_hashops !=
DistrHashOpsNotDeterminedYet)
{
GPOS_RAISE(
gpdxl::ExmaMD, gpdxl::ExmiMDObjUnsupported,
GPOS_WSZ_LIT(
"Query contains relations with a mix of default and legacy hash opclasses"));
}
m_context->m_distribution_hashops = DistrUseDefaultHashOps;
}
if (contains_legacy_hashops &&
m_context->m_distribution_hashops != DistrUseLegacyHashOps)
{
if (m_context->m_distribution_hashops !=
DistrHashOpsNotDeterminedYet)
{
GPOS_RAISE(
gpdxl::ExmaMD, gpdxl::ExmiMDObjUnsupported,
GPOS_WSZ_LIT(
"Query contains relations with a mix of default and legacy hash opclasses"));
}
m_context->m_distribution_hashops = DistrUseLegacyHashOps;
}
}
Expand Down Expand Up @@ -3934,7 +3952,9 @@ CTranslatorQueryToDXL::TranslateJoinExprInFromToDXL(JoinExpr *join_expr)
Node *join_alias_node = (Node *) lfirst(lc_node);
// rte->joinaliasvars may contain NULL ptrs which indicates dropped columns
if (!join_alias_node)
{
continue;
}
GPOS_ASSERT(IsA(join_alias_node, Var) ||
IsA(join_alias_node, FuncExpr) ||
IsA(join_alias_node, CoalesceExpr));
Expand Down
Loading

0 comments on commit e53b57c

Please sign in to comment.