Page MenuHomePhabricator

[clang] Remove duplication in types::getCompilationPhases()
ClosedPublic

Authored by thakis on Sep 29 2021, 1:18 PM.

Details

Summary

Call Driver::getFinalPhase() instead of duplicating it.

https://reviews.llvm.org/D65993 added the duplication, then
02e35832c301e maded it more obviously a copy of getFinalPhase().

The only difference is that getCompilationPhases() used to use
LastPhase / IfsMerge where getFinalPhase() used Link. Adapt
getFinalPhase() to return IfsMerge when needed.

No intentional behavior change.

Diff Detail

Event Timeline

thakis requested review of this revision.Sep 29 2021, 1:18 PM
thakis created this revision.
hans added inline comments.Sep 30 2021, 6:41 AM
clang/include/clang/Driver/Phases.h
25

Any reason not to keep the LastPhase alias?

thakis added inline comments.Sep 30 2021, 8:19 AM
clang/include/clang/Driver/Phases.h
25

I found it more confusing than helpful. Link and LastPhase both start with L and it took me a while to realize that the last return in getFinalPhase() was Link, but the last one in was getCompilationPhases(). For the call on line 3847, passing phases::IfsMerge is clearer. And then there's only one use of it left (the default arg), and it doesn't add a ton of clarity there imho. But maybe I subconsciously feel vengeful because I lost 10 minutes due to misreading it for Link :) If you want me to put it back, let me know and I'll do so.

hans accepted this revision.Sep 30 2021, 11:02 AM

lgtm

clang/include/clang/Driver/Phases.h
25

No, that sounds reasonable, let's keep your patch as it is.

This revision is now accepted and ready to land.Sep 30 2021, 11:02 AM
This revision was landed with ongoing or failed builds.Sep 30 2021, 11:17 AM
This revision was automatically updated to reflect the committed changes.