[PATCH] D111159: [UnknownProvenance] Introduce UnknownProvenance constant

Amaury SECHET via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 13 03:13:10 PDT 2021


deadalnix added inline comments.


================
Comment at: llvm/include/llvm-c/Core.h:1917
+ */
+LLVMValueRef LLVMGetUnknownProvenance(LLVMTypeRef Ty);
+
----------------
jeroen.dobbelaere wrote:
> deadalnix wrote:
> > Where is the implementation for these?
> Ah.. The implementation is in D111162, but I had to move the header earlier for the enum. Probably better to just add the implementation in this revision..
I'm not sure what the policy is here, but I would think that we should avoid adding stuff in the header when using them will only lead to a linker error.

But in this case, it seems that you have all the pieces to add this, at least the llvm/lib/IR/Core.cpp part.


================
Comment at: llvm/include/llvm/IR/Constants.h:579
+  /// which reduces the amount of casting needed in parts of the compiler.
+  inline PointerType *getType() const {
+    return cast<PointerType>(Value::getType());
----------------
jeroen.dobbelaere wrote:
> deadalnix wrote:
> > Isn't inline implied by the fact this is defined directly within the class?
> Yes it is. That's what I get for copy-pasting from `ConstantPointerNull`..
I think it'd be better to just remove it, as to not spread erroneous patterns (inline is notoriously misunderstood by many C/C++ devs). But either way, this is certainly not worth blocking that patch for this.


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

https://reviews.llvm.org/D111159



More information about the llvm-commits mailing list