dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land.
In D111105#3043734 <https://reviews.llvm.org/D111105#3043734>, @aeubanks wrote: > update test > I checked to make sure that we're not accepting -clear-ast-before-backend as > a driver flag Ah, OK - sorry I mistested some things when I was thinking about that. ================ Comment at: clang/test/Misc/clear-ast-before-backend.c:2-3 +// RUN: %clang -target x86_64-linux-gnu -c -Xclang -clear-ast-before-backend %s -S +// RUN: %clang -target x86_64-linux-gnu -c -Xclang -clear-ast-before-backend %s -S -### 2>&1 | FileCheck %s +// CHECK: "-clear-ast-before-backend" ---------------- dblaikie wrote: > This is a driver test, but not a very interesting one - it tests that -Xclang > arguments are passed directyl to cc1, which is probably tested elsewhere > already? > > I'm not sure there's anything we could really test with the first RUN line - > though since it's not a Driver test and doesn't need to be a Driver test - it > should probably just test cc1 -clear-ast-before-backend directly rather than > going through the driver+-Xclang Might be worth fleshing this out somehow to observe some behavior? I'm not sure exactly how - not like we'd generally test other memory optimizations we might make without a flag to opt into them. But perhaps a simple Hello World, maybe with some optimizations applied (so making this one of those rare "actually check it going through some optimizations" - I guess ever -O0 with an always_inline function or the like to demonstrate that the optimizations were applied, etc) Can't think of much else that wouldn't just be obnoxious in terms of memory usage - oh, unless there's some way to set an artificially low memory limit (some other tests might do this - clang/test/PCH/leakfiles.test puts a low ulimit for instance - some lldb and compiler-rt tests also set ulimits of various kinds to observe failures). An A/B test - showing some modest example hitting the memory limit without the flag, but passing with the flag, could be nice to have. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111105/new/ https://reviews.llvm.org/D111105 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
