This is an archive of the discontinued LLVM Phabricator instance.

Supress all uses of LLVM_END_WITH_NULL
ClosedPublic

Authored by serge-sans-paille on Apr 26 2017, 8:56 AM.

Details

Summary

Use variadic templates instead of relying on <cstdarg> + sentinel.

This enforces better type checking and makes code more readable.

No functional change.

Diff Detail

Repository
rL LLVM

Event Timeline

Note: I also have a patch to apply to clang because of the API chagne. What's the policy for cross-repo commits?

+ remove now useless <cstdarg> includes

mehdi_amini edited edge metadata.Apr 26 2017, 12:05 PM

Note: I also have a patch to apply to clang because of the API chagne. What's the policy for cross-repo commits?

Informal Policy is: you should update it, at least when it is about clang (a large part of the CI testing is bootstrapping the compiler).

(Side Note: this setup helps to do it atomically in one review / one commit : http://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo )

lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
479 ↗(On Diff #96795)

Why do you need the false here?

lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
479 ↗(On Diff #96795)

That's not related to this commit. the function signature is : `static FunctionType * get (const Type *Result, bool isVarArg)` so passing a casted null pointer was indeed working, but that's not very explicit :-/

mehdi_amini accepted this revision.May 9 2017, 8:12 AM

LGTM (provided you build/tested clang)

lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
479 ↗(On Diff #96795)

That's not related to this commit.

So commit it separately?
(you don't need a review for that)

This revision is now accepted and ready to land.May 9 2017, 8:12 AM
This revision was automatically updated to reflect the committed changes.

Forgot about polly which was broken by this. I fixed it in r302618.

@chandlerc thanks ! What's the minimal set of dependencies I should have checked in case of such API change?