[patch][rfc] Asserting that we have all use/users in the getters

[Teresa Johnson via llvm-commits]

On Fri, Dec 18, 2015 at 3:23 PM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> On 18 December 2015 at 10:29, Rafael Espíndola
> <rafael.espindola at gmail.com> wrote:
>> On 18 December 2015 at 09:17, Teresa Johnson <tejohnson at google.com> wrote:
>>> Also got bit by this once, so really like the idea.
>>>
>>> Some clear comments are needed to explain what "unchecked" means and
>>> when you would want to use it, since it isn't obvious from the name. I
>>> can't think of any alternate name that is particularly meaningful and
>>> reasonably concise. Maybe "materialized_uses", etc to indicate it is
>>> only looking for uses that have been materialized thus far?
>>
>> I like the name, thanks.
>>
>
> A renamed, rebased patch is attached.
>
> Cheers,
> Rafael



> +  // currently known given which functions are materiaziled. Be very careful

typo "materiaziled"


> --- a/lib/IR/Verifier.cpp
> +++ b/lib/IR/Verifier.cpp
> @@ -1831,7 +1831,7 @@ void Verifier::visitFunction(const Function &F) {
>
>   // If this function is actually an intrinsic, verify that it is only used in
>   // direct call/invokes, never having its "address taken".
> -  if (F.getIntrinsicID()) {
> +  if (F.getIntrinsicID() && !F.getParent()->getMaterializer()) {

Is this change necessary? Or just to avoid checking until after we
have all uses materialized?

Might be worth a comment. Also, maybe it would be clearer to have a
helper such as Module::ModuleIsMaterialized, to be used here and in
assertModuleIsMaterialized (which just asserts that it returns true).

Otherwise LGTM.

Thanks,
Teresa


-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413