[PATCH] D62265: [ScopBuilder] Move buildContext function from ScopInfo

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Thank you for your contribution. A few remarks for your first upload for Polly on Phabricator:

- Since these patches are mailed to llvm-commits, we were asked to add [Polly] into the title to make it immediately obvious that it's a patch for a subproject. The tag would be removed for the commit again.
- Append "NFC" (No Function Change intended) to the title for a refactoring patch. This one stays for the commit message.
- Add polly-dev (and llvm-commits) to subscribers.
- Add grosser to the reviewer list. He's code owner for Polly.

I am split with this patch. I generally think that we should avoid calling methods in constructors because it might be surprising which method is actually called. IMHO constructors are for initializing fields but that's exactly what `buildContext` does. Unfortunately, moving it to ScopBuilder means it directly accesses a foreign object's private fields. That's arguably worse. At some point we would like to drop the `friend class ScopBuilder` in ScopInfo such that such accesses are not possible anymore.

Methods that I think are better suited to be moved from ScopInfo to ScopBuilder:

- buildInvariantEquivalenceClasses
- buildDomains
- addUserAssumptions
- buildSchedule
- finalizeAccesses
- addUserContext
- addRecordedAssumptions
- buildAliasChecks
- hoistInvariantLoads
- canonicalizeDynamicBasePtrs
- verifyInvariantLoads


Repository:
  rPLO Polly

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62265/new/

https://reviews.llvm.org/D62265