hctim marked 2 inline comments as done. hctim added inline comments.
================ Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8849 break; - // HLSL attributes: ---------------- aaron.ballman wrote: > hctim wrote: > > aaron.ballman wrote: > > > Spurious whitespace change? > > unfortunately there's no way in my editor to trim trailing whitespace only > > on changed lines :(, so i end up fixing things like this drive-by. > > > > let me know if you feel very strongly about this diff and I can kill it, > > but I personally think the drive-by-fix isn't a huge problem and the > > alternative of whitespace-fix-only commit seems a bit overkill > Personally, I don't feel very strongly because the chances of the whitespace > being someone's entrypoint to git-blame is pretty minimal (especially given > there's only one change here). However, we typically still ask for formatting > changes to be separated out > (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access #2) > rather than lumped in with functional changes so reviewers will ask for these > sort of changes to be backed out, so this may crop up repeatedly if your > editor doesn't give you the options you need. Fixed in ee28837a1fbd574dbec14b9f09cb4effab6a492a. ================ Comment at: clang/test/CodeGen/hwasan-globals.cpp:1-2 +// RUN: echo "int extra_global;" > %t.extra-source.cpp +// RUN: echo "global:*ignorelisted_global*" > %t.ignorelist +// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=hwaddress -fsanitize-ignorelist=%t.ignorelist -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK ---------------- aaron.ballman wrote: > hctim wrote: > > aaron.ballman wrote: > > > Are these files automatically deleted when the test is done because we're > > > using %t, or do we need to clean those up manually? > > AFAIK nothing is ever automatically deleted (e.g. the outputs of the > > compiler). Is automated cleanup here necessary? > Necessary? Probably not (I'd expect these to go into the temp directory). A > kindness so disks don't fill up? Probably. Because the content here is static > anyway, these could just be files in the `Inputs` directory so they don't > need to be created every time the test is run. Done (and also will update `asan-globals.cpp` when this lands to use the new files as well). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127544/new/ https://reviews.llvm.org/D127544 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits