[PATCH] D23040: Tie the Verifier class to a Module; NFCI

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 1 17:15:56 PDT 2016


> On 2016-Aug-01, at 16:52, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> 
> sanjoy created this revision.
> sanjoy added reviewers: dexonsmith, bkramer, chandlerc.
> sanjoy added a subscriber: llvm-commits.
> Herald added a subscriber: mcrosier.
> 
> This commit changes the Verifier class to accept a Module via the
> constructor to make it obvious that a specific instance of the class is
> only intended to work with a specific module.  The `updateModule` setter
> (despite being private) was making this fact less transparent.
> 
> There are fields in the `Verifier` class like `DeoptimizeDeclarations`
> and `GlobalValueVisited` which are module specific, so a given
> Verifier instance will not in fact work across multiple modules today.
> This change just makes that more obvious.

Interesting.  This all SGTM then.

> I'm also tempted to change the `Verifier::verify(const Module &M)`
> function to `Verifier::verify()` (as a separate change perhaps?) if
> people are on-board with that.

Absolutely.  It doesn't make sense to take a Module here if it's going to be ignored.

I suggest doing it in the same change.

> 
> https://reviews.llvm.org/D23040
> 
> Files:
>  lib/IR/Verifier.cpp
> 
> <D23040.66402.patch>



More information about the llvm-commits mailing list