Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Verif] disambiguate ctor #8076

Merged

Conversation

makslevental
Copy link
Contributor

clang-tidy reports this ctor call as ambiguous and in fact it fails on Windows with clang-cl with

  D:\a\circt\circt\lib\Conversion\VerifToSV\VerifToSV.cpp(48,22): error: call to constructor of 'APInt' is ambiguous
     48 |         op.getLoc(), APInt(32, 0x80000001, false));
        |                      ^     ~~~~~~~~~~~~~~~~~~~~~
  D:\a\circt\circt\llvm\llvm\include\llvm/ADT/APInt.h(157,3): note: candidate constructor
    157 |   APInt(unsigned numBits, unsigned numWords, const uint64_t bigVal[]);
        |   ^
  D:\a\circt\circt\llvm\llvm\include\llvm/ADT/APInt.h(111,3): note: candidate constructor
    111 |   APInt(unsigned numBits, uint64_t val, bool isSigned = false,
        |   ^
  1 error generated.

@makslevental makslevental force-pushed the users/makslevental/disambig-ctor branch from 64ab27a to 59dbed2 Compare January 13, 2025 14:29
Copy link
Member

@maerhart maerhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting that this doesn't work properly on windows, but since 'false' is the default anyway this LGTM.

@makslevental
Copy link
Contributor Author

Interesting that this doesn't work properly on windows, but since 'false' is the default anyway this LGTM.

Presumably it does work on msvc.exe because you guys have tests? But yea not sure why clang-cl.exe complains (and then what's funnier/weirder is that it doesn't fail for clang on Linux/Mac 🤷).

@makslevental makslevental merged commit cdd3814 into llvm:main Jan 13, 2025
4 checks passed
@makslevental makslevental deleted the users/makslevental/disambig-ctor branch January 13, 2025 17:33
@makslevental
Copy link
Contributor Author

makslevental commented Jan 13, 2025

BTW there's actually another blocker on this path:


  lld-link: error: undefined symbol: public: __cdecl mlir::SuccessorRange::SuccessorRange(class mlir::Operation *)

  >>> referenced by D:\a\circt\circt\lib\Bindings\Python\ESIModule.cpp

  >>>               tools\circt\lib\Bindings\Python\CMakeFiles\CIRCTPythonModules.extension._circt.dso.dir\ESIModule.cpp.obj

  >>> referenced by D:\a\circt\circt\lib\Bindings\Python\MSFTModule.cpp

  >>>               tools\circt\lib\Bindings\Python\CMakeFiles\CIRCTPythonModules.extension._circt.dso.dir\MSFTModule.cpp.obj

  >>> referenced by D:\a\circt\circt\lib\Bindings\Python\SVModule.cpp

  >>>               tools\circt\lib\Bindings\Python\CMakeFiles\CIRCTPythonModules.extension._circt.dso.dir\SVModule.cpp.obj

but I'll come back to that one later because it's tougher to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants