mboehme added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp:67
+llvm::SmallVector<const InitListExpr *>
+allInitListExprForms(const InitListExpr *InitList) {
+ llvm::SmallVector<const InitListExpr *> result = {InitList};
----------------
PiotrZSL wrote:
> maybe some other name instead of allInitListExprForms, like getAllInitForms
How about this?
================
Comment at: clang-tools-extra/clang-tidy/utils/ExprSequence.cpp:69-72
+ if (InitList->isSemanticForm() && InitList->getSyntacticForm())
+ result.push_back(InitList->getSyntacticForm());
+ if (InitList->isSyntacticForm() && InitList->getSemanticForm())
+ result.push_back(InitList->getSemanticForm());
----------------
PiotrZSL wrote:
> this looks like typo... but it isn't, it's just unnecessary.
>
> ``InitList->isSemanticForm() && InitList->getSyntacticForm()`` is equal to:
> ``AltForm.getInt() && AltForm.getPointer()``
>
> ``InitList->isSyntacticForm() && InitList->getSemanticForm()`` is equal to:
> ``(!AltForm.getInt() || !AltForm.getPointer()) && (!AltForm.getInt()) &&
> AltForm.getPointer()``
>
> Why we coudn't have "getAnyForm".
>
> any code could just look like this:
> ``if (const InitListExpr * Form = InitList->getSyntacticForm())``
> `` result.push_back(Form);``
> ``if (const InitListExpr * Form = InitList->getSemanticForm())``
> `` result.push_back(Form);``
>
> this looks like typo... but it isn't, it's just unnecessary.
Ah, of course, thanks for pointing this out to me.
> Why we coudn't have "getAnyForm".
Not sure exactly what you mean?
I've updated the code to do what you suggested above (i.e. don't call
`isSemanticForm()`, just call `getSyntacticForm()` and see if returns non-null).
We still need to initialize `result` with `InitList` because
`InitList->getSyntacticForm()` returns null if `InitList` is already in
syntactic form.
I hope that makes sense? Please let me know if there's anything I missed from
your original response.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145906/new/
https://reviews.llvm.org/D145906
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
