User Details
- User Since
- Dec 9 2012, 11:41 PM (493 w, 4 d)
Tue, May 24
Fri, May 20
git-clang-format
The sema portions of this change are still causing an issue. Although the revert (rGf95bd18b5faa6a5af4b5786312c373c5b2dce687) and the subsequent re-application in rG3466e00716e12e32fdb100e3fcfca5c2b3e8d784 addressed the debug info issue, the Sema issue remains.
Thu, May 12
This seems reasonable to me. I think that a bit more exposition in the commit message is warranted though: explain why/how they are useful (i.e. they can be useful to validating the driver behaviour for path handling).
Wed, May 4
I wonder if we can just pay a penalty and simplify this to always just check the first member unconditionally.
Thanks!
Sun, May 1
Sat, Apr 30
Apr 26 2022
Apr 25 2022
Happy to help, thanks to Howard for the many years of stewardship!
Apr 23 2022
Sure, the argument of the cache hits is definitely reasonable - I'm not a fan of the naming, but can understand the desire to not rename all the variables across all the projects.
Apr 22 2022
Apr 21 2022
Apr 16 2022
Mar 16 2022
I don't have an example module sadly. It was something that I ran into with Swift code import the WinSDK module defined in https://github.com/apple/swift/blob/main/stdlib/public/Platform/winsdk.modulemap.
Mar 15 2022
This was something that I was hitting an issue with. In particular, it was a module build for a module which pulled in intsafe.h. Now, given that it is a preprocessor macro, it would stand to reason that it will normally be dropped and thus won't matter if the frontend doesn't actually support a 128-bit type. However, with a module, we would attempt to process the expression. I do admit it is on a slightly more tenuous ground as Microsoft may somehow do something different.
Mar 14 2022
Seems reasonable, though I'm not a fan of the variable names - they seem a bit difficult to read due to no separation (e.g., %fs-src-root or %fs_src_root vs %fssrcroot)
Mar 12 2022
Mar 11 2022
Mar 10 2022
Mar 9 2022
Mar 4 2022
Feb 15 2022
I believe that the break after ( in CMake is counter the style of the rest of the project. I'd recommend against it.
I'd like to see at least https://reviews.llvm.org/D108416#inline-1146215, https://reviews.llvm.org/D108416#inline-1146214, and https://reviews.llvm.org/D108416#inline-1146209 addressed pre-commit. Would be nice to have the other ones resolved as well, but they are more stylistic.
Feb 7 2022
Jan 31 2022
Jan 10 2022
Jan 7 2022
Jan 5 2022
Using the TARGET_OBJECTS: generator seems better. However, I also have concerns over using the shared library objects. At least for Windows, reuse of the objects meant for a shared object is not really correct. This is non-portable at best, and incorrect at worse. I would rather that we just introduce a proper target that we use can for embedding into another library (which may or may not expose its interfaces from the library that integrates it).
Jan 4 2022
Dec 28 2021
Dec 21 2021
Dec 13 2021
Thanks @thakis - 906e60b9f923464cba0f71a9205846550752162f should have addressed that from a few days ago (sorry about not mentioning that here)
Dec 8 2021
Dec 4 2021
I think that this is almost correct now. While this will generate the binaries correctly, this is still a problem for tests. Without the absolute path, we now run the risk of the DT_NEEDED being correct but without a DT_RPATH/DR_RUNPATH, this could pick up the wrong library still at runtime. While the link time issue is resolved, the runtime portion is still a problem.
Nov 29 2021
Nov 28 2021
Add missing null-terminators.
Fix build rules
This is a more complete implementation that allows for error reporting and use of UCS-2 as that is needed for the search path alteration.
I'm not sure that this is a good idea. This now means that you can link against two different versions of this library - one when building LLVMSupport and one that any user linking against LLVMSupport happens to find in the library search path. Consider someone explicitly providing a path to the terminfo library when configuring. At the very least, the library search path should be propagated to ensure that the right version is used.
Nov 24 2021
Nov 9 2021
Nov 8 2021
@craig.topper - ping
Nov 1 2021
Oct 31 2021
Oct 26 2021
@mstorsjo I think that it might be interesting to consider a wrapper for enable_languages and to split that out from project.
I think that as long as the builders remain green, this is fine. Yes, this is very likely to break users. We should announce this change on llvm-dev at the very least, and you *could* possibly add a transition helper in the form of:
Oct 16 2021
I think that we might really want to subsequently rework the compiler-rt handling to use target_compile_options. This munging of flags manually is really rather onerous, but that is beyond the scope of this change.
Sep 20 2021
While I think that the direction of this change is amazing, I'm less enthusiastic about the naming. I think that it is particularly confusing for the flags to use C and CXX as the prefixes for the flags as we have llvm-libc (aka "c") and libc++ (aka "c++", "cxx"). Using a more verbose name of COMPILER_ or C_COMPILER_, CXX_COMPILER_ would allow the flags to be easily attributed to the compiler or the runtime. However, the C standard flags definitely should be dropped.
Sep 16 2021
Sep 15 2021
Sep 14 2021
@thakis - thanks, seems that I had a part of the change sitting in my stash ... I had added a -NOT to verify the behaviour, and forgot to remove it. I'll revert the revert with the fix.
@Eugene.Zelenko - sorry, I didn't see the additional comments before the commit. I'm happy to do a follow up depending on the resolution.
Sep 13 2021
Address review feedback
Sep 8 2021
As to the comment on the consistency - there are a number of different layouts that are inconsistent with each other :-(. This is messy, and therefore forcing the user to specify the paths seems like a good compromise.
I really would prefer to avoid adding bin/<TRIPLE> to the default installation path. This doesn't match the defaults for most platforms (e.g. exherbo uses /usr/<triple>/bin - which would be a default value of ${LLVM_DEFAULT_TARGET_TRIPLE}/bin. On Windows, it seems that bin, bin64, bin64a is the style that Microsoft uses. Having to have different defaults seems unfortunate. Defaulting to bin and letting the user override it seems reasonable to me.