[PATCH] D103773: [clang-cl] Add /permissive and /permissive-

Sat Jun 19 04:30:51 PDT 2021

zero9178 marked an inline comment as done.
zero9178 added inline comments.

================
Comment at: clang/test/Driver/cl-permissive.c:15
+// PERMISSIVE-OVERWRITE-NOT: "-fdelayed-template-parsing"
+// RUN: %clang_cl /permissive- /Zc:twoPhase- -### -- %s 2>&1 | FileCheck -check-prefix=PERMISSIVE-MINUS-OVERWRITE %s
+// PERMISSIVE-MINUS-OVERWRITE-NOT: "-fno-operator-names"
----------------
zero9178 wrote:
> aganea wrote:
> > Hello @zero9178 !
> > After this patch, the following fails:
> > `// RUN: %clang_cl /permissive- -- %s`
> > generates:
> > `error: error reading '/Zc:strictStrings'`
> > Because `/Zc:strictStrings` isn't correctly expanded to `-Werror=c++11-compat-deprecated-writable-strings` as it should, but it is passed as-is to cc1. You can put a breakpoint here: https://github.com/llvm/llvm-project/blob/fc018ebb608ee0c1239b405460e49f1835ab6175/clang/lib/Driver/ToolChains/Clang.cpp#L5374 and observe before/after this line.
> > Would you possibly have a chance to take a look please?
> Absolutely! I will take a look as soon as possible which will be on the weekend. Meanwhile feel free to either revert the commit or remove the expansion to /Zc:strictStrings
I have removed the expansion to strictStrings in https://reviews.llvm.org/rGc9889c44ec5a4054833457c813e155f284703ef4. Apparently when AdAllArgs is called it does not correctly render it as I thought it would. The fix isn't very obvious to me, and my time is sadly limited. In hindsight it is also probably not a good idea to add strictStrings until strictStrings exists/is neeeded. 

Hope this helps.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103773/new/

https://reviews.llvm.org/D103773