Page MenuHomePhabricator

[SyntaxTree] Improve safety of `replaceChildRangeLowLevel`
ClosedPublic

Authored by eduucaldas on Oct 13 2020, 2:47 AM.

Details

Summary
  • Add assertions for other preconditions.
  • If nothing is modified, don't mark it.

Diff Detail

Event Timeline

eduucaldas created this revision.Oct 13 2020, 2:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 13 2020, 2:47 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
eduucaldas requested review of this revision.Oct 13 2020, 2:47 AM
eduucaldas added inline comments.
clang/lib/Tooling/Syntax/Tree.cpp
103

Throughout the function we use data members instead of accessors. Is one preferrable to the other?

gribozavr2 added inline comments.Oct 13 2020, 11:00 AM
clang/lib/Tooling/Syntax/Tree.cpp
103

I don't think there is a difference.

124

Why lambda and not:

Node *&Begin = BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
eduucaldas marked 2 inline comments as done.

Answer to comments

gribozavr2 accepted this revision.Oct 14 2020, 2:17 AM
gribozavr2 added inline comments.
clang/lib/Tooling/Syntax/Tree.cpp
122

Could you move this definition up so that it can be used in the last assert above?

This revision is now accepted and ready to land.Oct 14 2020, 2:17 AM
This revision was landed with ongoing or failed builds.Oct 14 2020, 2:21 AM
This revision was automatically updated to reflect the committed changes.
eduucaldas marked an inline comment as done.Oct 14 2020, 2:42 AM
eduucaldas added inline comments.
clang/lib/Tooling/Syntax/Tree.cpp
122

Done in a separate commit: 6fbad9bf304c05d37454420f7d5a1c2ab3adab20