tra added a comment.
@jyknight - James, do you have further concerns about the patch?
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6454
+ bool DiagAtomicLibCall = true;
+ for (auto *A : Args.filtered(options::OPT_W_Joined)) {
+ if (StringRef(A->getValue()) == "no-error=atomic-alignment")
----------------
If we rely on promoting the warnings to errors for correctness, I think we may
need a more robust mechanism to enforce that than trying to guess the state
based on provided options.
E.g. can these diagnostics be enabled/disabled with a wider scope option like
`-W[no-]extra` or `-W[no-]all`?
Maybe we should add a cc1-only option `--enforce-atomic-alignment` and use that
to determine if misalignment should be an error at the point where we issue the
diagnostics?
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6457
+ DiagAtomicLibCall = false;
+ if (StringRef(A->getValue()) == "error=atomic-alignment")
+ DiagAtomicLibCall = true;
----------------
This should be `else if`, or, maybe use `llvm::StringSwitch()`instead:
```
DiagAtomicLibCall = llvm::StringSwitch<bool>(A->getValue())
.Case("no-error=atomic-alignment", false)
.Case("error=atomic-alignment", true)
.Default(DiagAtomicLibCall)
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71726/new/
https://reviews.llvm.org/D71726
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
