[clang][diagnostics] Optionally enforce explicit stable IDs#3
Draft
[clang][diagnostics] Optionally enforce explicit stable IDs#3
Conversation
steakhal
reviewed
Dec 17, 2025
steakhal
left a comment
steakhal
left a comment
There was a problem hiding this comment.
I had a quick and shallow look and it looked good to me so far.
dbartol
pushed a commit
that referenced
this pull request
Mar 2, 2026
Using code/ideas from the x86 backend to optimize a select on a bitcast integer. The previous aarch64 approach was to individually extract the bits from the mask, which is kind of terrible. https://rust.godbolt.org/z/576sndT66 ```llvm define void @if_then_else8(ptr %out, i8 %mask, ptr %if_true, ptr %if_false) { start: %t = load <8 x i32>, ptr %if_true, align 4 %f = load <8 x i32>, ptr %if_false, align 4 %m = bitcast i8 %mask to <8 x i1> %s = select <8 x i1> %m, <8 x i32> %t, <8 x i32> %f store <8 x i32> %s, ptr %out, align 4 ret void } ``` turned into ```asm if_then_else8: // @if_then_else8 sub sp, sp, llvm#16 ubfx w8, w1, llvm#4, #1 and w11, w1, #0x1 ubfx w9, w1, llvm#5, #1 fmov s1, w11 ubfx w10, w1, #1, #1 fmov s0, w8 ubfx w8, w1, llvm#6, #1 ldp q5, q2, [x3] mov v1.h[1], w10 ldp q4, q3, [x2] mov v0.h[1], w9 ubfx w9, w1, #2, #1 mov v1.h[2], w9 ubfx w9, w1, #3, #1 mov v0.h[2], w8 ubfx w8, w1, llvm#7, #1 mov v1.h[3], w9 mov v0.h[3], w8 ushll v1.4s, v1.4h, #0 ushll v0.4s, v0.4h, #0 shl v1.4s, v1.4s, llvm#31 shl v0.4s, v0.4s, llvm#31 cmlt v1.4s, v1.4s, #0 cmlt v0.4s, v0.4s, #0 bsl v1.16b, v4.16b, v5.16b bsl v0.16b, v3.16b, v2.16b stp q1, q0, [x0] add sp, sp, llvm#16 ret ``` With this PR that instead emits ```asm if_then_else8: adrp x8, .LCPI0_1 dup v0.4s, w1 ldr q1, [x8, :lo12:.LCPI0_1] adrp x8, .LCPI0_0 ldr q2, [x8, :lo12:.LCPI0_0] ldp q4, q3, [x2] and v1.16b, v0.16b, v1.16b and v0.16b, v0.16b, v2.16b ldp q5, q2, [x3] cmeq v1.4s, v1.4s, #0 cmeq v0.4s, v0.4s, #0 bsl v1.16b, v2.16b, v3.16b bsl v0.16b, v5.16b, v4.16b stp q0, q1, [x0] ret ``` So substantially shorter. Instead of building the mask element-by-element, this approach (by virtue of not splitting) instead splats the mask value into all vector lanes, performs a bitwise and with powers of 2, and compares with zero to construct the mask vector. cc rust-lang/rust#122376 cc llvm#175769
llvm#175003 Create public function and entrypoints for iswdigit and add UTF8 test to iswalpha test. On CI there is no UTF8 build currently, so that test case will only run locally, but [wctype_utils_test.cpp](https://github.com/llvm/llvm-project/blob/main/libc/test/src/__support/wctype_utils_test.cpp) is written in a way that both ASCII and UTF8 tests run regardless of the build config.
…(NFC)" (llvm#185372) Reverts llvm#184572 Breaks many buildbots.
…vm#185371) This brings it inline with other intrinsics that require rvv
… members" (llvm#184804) Use `getElementAsAPInt(`) to read array elements and emit via `addIntAsBlock()` which handles target endianness correctly, instead of `getRawDataValues()` which exposes host endianness. This fixes test failures on big-endian hosts cross-compiling for little-endian targets. Reland of llvm#182442 with endianness fix.
… with analysis (llvm#185233) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp had spelling mistake in error message string. This PR fixes the spelling mistake and replaces 'anaylsis' with 'analysis'
- Empty functions (with no blocks) should are skipped by partition pass, blocks with more than one continue to get error-flagged - fixed ShardingInterfaceImpl of bufferization.materialize_in_destination
…ttr (llvm#185284) The name field of DICompositeTypeAttr was already optional, but DIBasicTypeAttr and DIStringTypeAttr were not handled consistently. Make the name parameter of both an OptionalParameter to support LLVM debug info nodes with no name. Update DebugImporter to use getStringAttrOrNull when translating the name of these types. Add tests for the null-name cases in the import and export test files.
To be consistent with the other comments in the same test.
This was emitting the raw rcp intrinsic, which will fail for any vector type. This is an afn context anyway, so just emit fdiv which will select to rcp but also will undergo type legalization.
This duplicates llvm#182313 with some very small modifications on top, as @dheaton-arm is unable to finish the PR and I'm unable to push to his branch. Expands support for the `FindLast` Reccurence Kind to floating-point and pointer types, thereby enabling conditional scalar assignment (CSA) for these types. Originally authored by @dheaton-arm --------- Co-authored-by: Damian Heaton <Damian.Heaton@arm.com>
…part 27) (llvm#185340) Tests converted from test/Lower/Intrinsics: date_and_time.f90, dconjg.f90, dim.f90, dimag.f90, dprod.f90
amdgcn-- was probably dead when clover was being maintained, since it switched to using amdgcn-mesa-mesa3d. Also remove amdgcn-mesa-mesa3d, since clover is no longer in mesa.
…e= options (llvm#184421) Along the way, it also fixes the static registration of the builtin (upstream) formats and extractors.
On PPC64, SIGTRAP is delivered before the triggering instruction completes. The previous implementation wrongly assumed that the instruction gets executed and due to that qHostInfo has the wrong info, causing the SingleStep being avoided on Linux for PPC64, and the execution being stuck at the store instruction. This patch corrects this, making watchpoint work.
…mary-file= options" (llvm#185391) Reverts llvm#184421 Broke build bot: https://lab.llvm.org/buildbot/#/builders/204/builds/38459/steps/7/logs/stdio ``` [7440/8486] Linking CXX shared library lib/libclangAnalysisScalable.so.23.0git FAILED: lib/libclangAnalysisScalable.so.23.0git : && /usr/bin/c++ -fPIC -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-array-bounds -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -O3 -DNDEBUG -Wl,-z,defs -Wl,-z,nodelete -Wl,-rpath-link,/home/botworker/bbot/amdgpu-offload-rhel-8-cmake-build-only/build/./lib -Wl,--gc-sections -shared -Wl,-soname,libclangAnalysisScalable.so.23.0git -o lib/libclangAnalysisScalable.so.23.0git tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/ASTEntityMapping.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/EntityLinker/EntityLinker.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Frontend/TUSummaryExtractorFrontendAction.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Model/BuildNamespace.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Model/EntityId.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Model/EntityIdTable.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Model/EntityLinkage.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Model/EntityName.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Model/SummaryName.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Serialization/JSONFormat/JSONFormatImpl.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Serialization/JSONFormat/LUSummary.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Serialization/JSONFormat/LUSummaryEncoding.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Serialization/JSONFormat/TUSummary.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Serialization/JSONFormat/TUSummaryEncoding.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Serialization/SerializationFormatRegistry.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Support/ErrorBuilder.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/TUSummary/ExtractorRegistry.cpp.o tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/TUSummary/TUSummaryBuilder.cpp.o -Wl,-rpath,"\$ORIGIN/../lib:/home/botworker/bbot/amdgpu-offload-rhel-8-cmake-build-only/build/lib:" lib/libclangIndex.so.23.0git lib/libclangFrontend.so.23.0git lib/libclangASTMatchers.so.23.0git lib/libclangAST.so.23.0git lib/libclangLex.so.23.0git lib/libclangBasic.so.23.0git lib/libLLVMSupport.so.23.0git -Wl,-rpath-link,/home/botworker/bbot/amdgpu-offload-rhel-8-cmake-build-only/build/lib && : tools/clang/lib/Analysis/Scalable/CMakeFiles/obj.clangAnalysisScalable.dir/Frontend/TUSummaryExtractorFrontendAction.cpp.o:(.data.rel.ro._ZTVN12_GLOBAL__N_115TUSummaryRunnerE+0xc0): undefined reference to `clang::SemaConsumer::anchor()' collect2: error: ld returned 1 exit status ```
When devices are not properly initialized llvm-gpu-loader follows corrupt pointers which result in hard to debug crashes. This improves the checks to avoid such situations.
…izes (llvm#185098) These were assuming uniform work group sizes. Emit the v4 and v5 sequences to take the remainder group for the nonuniform case. Currently the device libs uses this builtin on the legacy ABI path with the same sequence to calculate the remainder, and fully implements the v5 path. If you perform a franken-build of the library with the updated builtin, the result is worse. The duplicate sequence does not fully fold out. However, it does not appear to be wrong. The relevant conformance tests still pass.
Replace `@TODO` with `TODO` to follow LLVM comment conventions. This is an NFC change — no functional impact.
kaleidoscope chapter 03 explanation has this function redefine check, but it was missing in the code sample. Signed-off-by: amila <amila.15@cse.mrt.ac.lk>
At least the ones we visit directly via the visitor. This was always the case, except for BinaryOperator and CastExpr.
… with index type (llvm#183652) `buildTernaryFn` for `TernaryFn::select` called `llvm_unreachable` when the operand types were not `i1`, integer, or floating-point (e.g., `index` type). Instead delegate this checking to the IR verifier: we don't need to duplicate the checks from the verifier in an assertion here. Fixes llvm#179046 Assisted-by: Claude Code
…vm#185187) Move the `llvm-strings` test dependency into the non-standalone test dependency block, to fix standalone builds after llvm#182846. While at it, reformat the block to make it more visible. Signed-off-by: Michał Górny <mgorny@gentoo.org>
) Implement the missing CIR lowerings for the AdvSIMD (Neon) `vabd_*` (absolute difference) intrinsic group. Most `vabd` variants were already supported (see llvm#183595); this patch completes the remaining cases listed in [1]. Move the corresponding tests from: * clang/test/CodeGen/AArch64/neon_intrinsics.c to: * clang/test/CodeGen/AArch64/neon/intrinsics.c The implementation mirrors the existing lowering in CodeGen/TargetBuiltins/ARM.cpp. To support this, add the `emitCommonNeonSISDBuiltinExpr` helper. Reference: [1] https://arm-software.github.io/acle/neon_intrinsics/advsimd.html#absolute-difference
…ull (llvm#185513) Our current implementation of string lowering did some work to remove extra trailing zeros, plus do a 'zero' constant. That is unchanged by this patch. However, this patch ALSO ensures that we do the 'remove extra trailing zeros' to remove ALL trailing zeros, which likely has canonicalization benefits later on. However, the real benefit of this patch is to make string emission by default emit a null-terminator, which fixes the virtual table 'name' field get lowered correctly. We do this by making the builder::getString function take an argument (true by default) that will ensure we add a null terminator if necessary. This reflects the llvm::ConstantDataArray::getString function, which has the same functionality. However, doing this during lowering seems incorrect, since the FE is the one that knows whether these null terminators are necessary. There is not currently an 'opt out' use of the behavior, but the functionality is left in place to better reflect the ConstantDataArray::getString function interface. Note with the tests that this fixes some inconsistencies between LLVM and OGCG lowering.
Add the definitions of the "flatten" and the "split" constructs to the OMP.td file. This will allow the implementation efforts in clang and flang to proceed independently. There is no other functionality added in this patch. The "flatten" construct is defined in the OpenMP Technical Report 14: https://www.openmp.org/wp-content/uploads/openmp-TR14.pdf
…n and cos with 1 ULP errors. (llvm#184752) Size of `sin` for armv8m: Before the patch: ``` $ ls -l libc/src/math/generic/CMakeFiles/libc.src.math.generic.sin.dir/ total 16 -rw-r----- 1 lntue primarygroup 13408 Mar 5 07:38 sin.cpp.obj $ llvm-nm-19 --radix=d --print-size --size-sort --reverse-sort libc/src/math/generic/CMakeFiles/libc.src.math.generic.sin.dir/sin.cpp.obj 00000000 00002048 V _ZN22__llvm_libc_23_0_0_git4math31range_reduction_double_internal24ONE_TWENTY_EIGHT_OVER_PIE 00000000 00001632 W _ZN22__llvm_libc_23_0_0_git4math31range_reduction_double_internal19LargeRangeReduction4fastEdRNS_10NumberPairIdEE 00000000 00001412 W _ZN22__llvm_libc_23_0_0_git4math3sinEd 00000000 00001048 W _ZN22__llvm_libc_23_0_0_git4math20sincos_eval_internal11sincos_evalERKNS_10NumberPairIdEERS3_S6_ 00000000 00001040 V _ZN22__llvm_libc_23_0_0_git4math31range_reduction_double_internal17SIN_K_PI_OVER_128E 00000000 00000528 W _ZN22__llvm_libc_23_0_0_git4math31range_reduction_double_internal21range_reduction_smallEdRNS_10NumberPairIdEE 00000000 00000004 T sin 00000000 00000004 V _ZZN22__llvm_libc_23_0_0_git6fputil7generic15quick_get_roundEvE1x 00000000 00000004 T _ZN22__llvm_libc_23_0_0_git3sinEd U __aeabi_memclr8 U __aeabi_dsub U __aeabi_dmul U __aeabi_dcmplt U __aeabi_dcmpgt U __aeabi_dcmpeq U __aeabi_dadd U __aeabi_d2lz ``` After the patch: ``` $ ls -l libc/src/math/generic/CMakeFiles/libc.src.math.generic.sin.dir/ total 8 -rw-r----- 1 lntue primarygroup 5424 Mar 5 07:35 sin.cpp.obj $ llvm-nm-19 --radix=d --print-size --size-sort --reverse-sort libc/src/math/generic/CMa keFiles/libc.src.math.generic.sin.dir/sin.cpp.obj 00000000 00002408 W _ZN22__llvm_libc_23_0_0_git4math12integer_only3sinEd 00000000 00000332 W _ZNK22__llvm_libc_23_0_0_git4math12integer_only7Frac128mlERKS2_ 00000000 00000167 r .L__const._ZN22__llvm_libc_23_0_0_git4math12integer_only3sinEd.TWO_OVER_PI 00000000 00000096 r .L__const._ZN22__llvm_libc_23_0_0_git4math12integer_only3sinEd.SIN_COEFF 00000000 00000096 r .L__const._ZN22__llvm_libc_23_0_0_git4math12integer_only3sinEd.COS_COEFF 00000000 00000004 T sin 00000000 00000004 T _ZN22__llvm_libc_23_0_0_git3sinEd ```
Inline a variable in an assertion given it only has a single use.
…amounts (llvm#183669) 64-bit regpair shift amounts are treated as signed 7-bits, so a complement shift amount of 64 (when the primary amount is 0) is sign-extended to -64, reversing the shift direction and producing incorrect results. This affected any 64-bit rotate or funnel shift where the runtime shift amount could be 0 (making the complement 64) or >= 64. Fix by masking the shift amount to [0, 63] and computing the complement as (m - 64), which is always in [-64, -1]. Using lsl/lsr (logical shift) instructions with this negative amount causes the hardware to reverse the shift direction while zero-filling vacated positions: fshl(a, b, amt) = (a << m) | lsl(b, m - 64) // lsl reverses to lsr fshr(a, b, amt) = (b >> m) | lsr(a, m - 64) // lsr reverses to lsl where m = amt & 63. The logical shift instructions (lsl/lsr) are used instead of arithmetic (asl) because asl with a negative amount performs an arithmetic right shift that sign-extends, which would corrupt the result for negative source values. When m = 0, the complement amount is -64 (magnitude 64), which shifts all 64 bits out and produces zero, so the complement term vanishes as required.
… ExecutableScriptingResourcesFromDSYM warning message (llvm#185640) About to make changes in this area and using `formatv` instead of `printf` style format specifiers makes those easier to follow.
Changes include: - Added iswlower entrypoint in wctype.yaml to expose the function - Created iswlower.h header and iswlower.cpp implementation - Added CMake entrypoint object for iswlower - Created unit test in iswlower_test.cpp - Added test entry to wctype CMakeLists.txt this PR helps in exposing iswlower which internally calls islower on wide character built using : ninja -C build libc tested using : ninja libc_wctype_unittests and all the 3 tests passed resolves issue llvm#185136
…m#185644) Compare registers using their enum values instead, which I suspect was the intention in the first place, since we already have lexicographical ordering defined for CodeGenRegisters. This does not cause any changes in .inc files and is likely NFC, but it's still best to have it be deterministic.
…tPointers pass (llvm#184662) After the change from `report_fatal_error` to `Ctx.emitError` in llvm#142014 there is a necessity to remove unsupported globals. Otherwise there is a secondary crash during ISel when processing them Fixes SWDEV-511241
…5076) This is a relatively simple strategy as it is omitting any heuristics for liveness and register pressure reduction. This works well as the SystemZ ISel scheduler is using Sched::RegPressure which gives a good input order to begin with. It is trying harder with biasing phys regs than GenericScheduler as it also considers other instructions such as immediate loads directly into phys-regs produced by the register coalescer. This can hopefully be refactored into MachineScheduler.cpp. It has a latency heuristic that is slightly different from the one in GenericScheduler: It is activated for a specific type of region that have many "data sequences" consisting of SUs connected only with a single data-edge that are next to each other in the input order. This is only 3% of all the scheduling regions, but when activated it is applied on all the candidates (not just once per cycle). At the same time it is a bit more careful by checking not only the SU Height against the scheduled latency but also its Depth against the remaining latency. It reuses the GenericScheduler handling of weak edges to help copy coalescing. It also helps with compare zero elimination as it tries to put a CC-defining instruction that produces the compare source value above the compare before any other instruction clobbering CC or the value. This work was started after observing heavy spilling in Cactus, which was actually *caused* by GenericScheduler - disabling it (no pre-RA scheduling) remedied it and gave a 7% improvement in performance on that benchmark. Many different versions have been tried which has evolved into this initial simplistic MachineSchedStrategy that does relatively little and yet achieves double-digit improvements on Cactus and Imagick compared to GenericSched (which is OTOH 3% better on Blender). There will hopefully be more improvements added later on as there seems to be potential for it. It would be very interesting to have other OOO targets try this as well and perhaps make this available in MachineScheduler.cpp (A first attempt with improving the pre-RA scheduling was made with llvm#90181, which however did not materialize in anything actually useful.)
Introduced validation for the `dmask` argument of the aforementioned built-ins to match LLVM IR verifier behavior that is being changed in llvm#179511.
…ed assertion I'm about to reword the error message. Having test coverage for the message will make that change easier to review/reason about.
…185074) Patch to add custom lowering for FCANONICALIZE, FMAXNUM_IEEE, and FMINNUM_IEEE, all of which are required when relying on default expansion of FMAXIMUMNUM and FMINIMUMNUM. The lowering is very simple because AArch64's FMAXNM and FMINNM instructions are IEEE754-2008 compliant, with the implementation effectively follow the same path take for NEON. NOTE: Bfloat support will be provided separately.
This fixes 05d96d5.
…part 28) (llvm#185549) Tests converted from test/Lower/Intrinsics: dreal.f90, dshiftl.f90, dshiftr.f90, eoshift.f90, erfc_scaled.f90
…lvm#185439) Function `checkAuthenticatedLR` was declared but not defined anywhere. This patch removes the dangling declaration.
Otherwise multiple translation units in the same process could run into ID reuse collisions cause invalid SPIR-Vs to be generated due to having multiple definition for the same SPIR-V SSA value. Closes: llvm#160613
Patch fixes llvm.org/pr20231. The original test was expecting clock() to return 0 when stepping in debugger which in reality can never happen.
Implement iswcntrl entrypoint and test for llvm#185136
…red with SYCL attributes. (llvm#185522) Commit cf6cc66 ([OpenMP][SYCL] Improve diagnosing of unsupported types usage) customized `Sema::getEmissionStatus()` to return `Emitted` for a function declared with the `sycl_kernel` attribute during device compilation. That change is appropriate, but was inserted before a check for a dependent context and resulted in `Emitted` being returned instead of `TemplateDiscarded` for templated functions declared with the attribute. That appears to be incorrect; templated functions are still discarded. The customization was extended to include the `sycl_kernel_entry_point` and `sycl_external` attributes in commit 23e4fe0 ([SYCL] SYCL host kernel launch support for the sycl_kernel_entry_point attribute). Those additions are appropriate, but the effect on templated functions (as opposed to their instantiations) resulted in the incorrect status being observed in a downstream fork of Clang. This change corrects `Sema::getEmissionStatus()` to once again unconditionally return `TemplateDiscarded` for templated functions.
…lvm#185659) Fixes a verification failure in X86SelectionDAGInfo::verifyTargetNode (llvm#185649)
Currently many comments parsed from the AST contain leading spaces. Trim the leading and trailing spaces before using them in clang-doc.
… By-Name Scanning (llvm#183396) (llvm#185474) This PR fixes two issues of the in-memory buffer we use for the input file when a dependency scanner performs by-name queries. First, it renames the buffer. The temporary file was named `ScanningByName-%%%%%%%%.input`, which leads to weird diagnostics such as ``` ScanningByName-2d42a1e9.input:1:1: fatal error: could not build module 'X' ``` This PR changes the name of the file buffer, so we get diagnostics such as ``` module-include.input:1:1: fatal error: could not build module 'X' ``` which is more indicative. Additionally, this PR fixes a bug where the source location may overflow the temporary buffer by creating a 64k empty string which the temporary buffer occupies. When the source location overflows, the diagnostics could point to some random file that comes after the fake file and is incorrect. Currently, the maximum number of unique names from Apple's SDKs is around 3000. A 64k buffer per dependency scanning worker gives us around 20x capacity per worker (which scans fewer names than 3000 when the scanning is done in parallel). A fatal error is added to catch overflows.
This PR adds support to `clang-tblgen` to enforce that all warnings and extensions have explicitly-specified stable IDs, based on [discussion](llvm#168153 (comment)) in a previous PR. The clang-tblgen option to control this is `-enforce-stable-ids=<error|warning|none>`. To control this as part of the Clang build, specify `CLANG_ENFORCE_STABLE_IDS=<error|warning>` to CMake. The default, for now, is to not enforce explicit stable IDs at all. I'll ramp this up to a warning in an upcoming PR, where I will also add stable IDs for all existing upstream warnings and extensions. We should give downstream forks some time to fix up their private diagnostics before we start enforcing explicit stable IDs with an error. Clang-tblgen will emit fix suggestions for missing stable IDs, both as fixits to the console and as a clang-tidy-style YAML file to a path specified by `-export-fixes=`. The fixes from the entire Clang build can be bulk-applied via clang-apply-replacements; clang-tblgen will emit an additional warning specifying the exact command to run. Re-using the existing YAML diagnostic/fixit support required a bit of minor refactoring. The `clang::tooling` library that contains that support also depends on parts of Clang itself, which would introduce a circular dependency if used in clang-tblgen. The cleanest way I could find to make this work was to ensure that the subset of the tooling library that is now needed in clang-tblgen does not `#include` any Clang headers. Those tooling headers _do_ still depend on some Clang types, but not for the subset of functionality used by clang-tblgen. So, I just used forward declarations of those Clang types, and as long as clang-tblgen doesn't call the affected functions, clang-tblgen will still compile. To make the consumption from clang-tblgen header-only, I had to move a few simple functions to be `inline` in the header as well. The actual changes to clang-tblgen are pretty straightforward, although I did have to plumb the support for YAML fixits through from the command line. Most of the remaining changes are adding explicit `llvm::` qualifications that are now needed because one of the Clang headers that is no longer being included must have had a stray `using namespace llvm;`.
dbartol
force-pushed
the
sarif/warn-stable-ids
branch
from
adac1e9 to
237d1d6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds support to
clang-tblgento enforce that all warnings and extensions have explicitly-specified stable IDs, based on discussion in a previous PR. The clang-tblgen option to control this is-enforce-stable-ids=<error|warning|none>. To control this as part of the Clang build, specifyCLANG_ENFORCE_STABLE_IDS=<error|warning>to CMake.The default, for now, is to not enforce explicit stable IDs at all. I'll ramp this up to a warning in an upcoming PR, where I will also add stable IDs for all existing upstream warnings and extensions. We should give downstream forks some time to fix up their private diagnostics before we start enforcing explicit stable IDs with an error.
Clang-tblgen will emit fix suggestions for missing stable IDs, both as fixits to the console and as a clang-tidy-style YAML file to a path specified by
-export-fixes=. The fixes from the entire Clang build can be bulk-applied via clang-apply-replacements; clang-tblgen will emit an additional warning specifying the exact command to run.Re-using the existing YAML diagnostic/fixit support required a bit of minor refactoring. The
clang::toolinglibrary that contains that support also depends on parts of Clang itself, which would introduce a circular dependency if used in clang-tblgen. The cleanest way I could find to make this work was to ensure that the subset of the tooling library that is now needed in clang-tblgen does not#includeany Clang headers. Those tooling headers do still depend on some Clang types, but not for the subset of functionality used by clang-tblgen. So, I just used forward declarations of those Clang types, and as long as clang-tblgen doesn't call the affected functions, clang-tblgen will still compile. To make the consumption from clang-tblgen header-only, I had to move a few simple functions to beinlinein the header as well.The actual changes to clang-tblgen are pretty straightforward, although I did have to plumb the support for YAML fixits through from the command line.
Most of the remaining changes are adding explicit
llvm::qualifications that are now needed because one of the Clang headers that is no longer being included must have had a strayusing namespace llvm;.