tlively added inline comments.
================ Comment at: lib/Driver/ToolChains/WebAssembly.cpp:29 + Pthread = + DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false); + ThreadModel = ---------------- Shouldn't every use of `hasFlag` be `getLastArgValue` instead? ================ Comment at: lib/Driver/ToolChains/WebAssembly.cpp:36 + // Did user explicitly specify -mthread-model / -pthread? + bool HasThreadModel = DriverArgs.hasArg(options::OPT_mthread_model); + bool HasPthread = Pthread && DriverArgs.hasArg(options::OPT_pthread); ---------------- It doesn't matter whether the user included the -pthread flag if they later included the -no-pthread flag. ================ Comment at: lib/Driver/ToolChains/WebAssembly.cpp:42 + // "posix") + if (HasPthread && HasThreadModel && ThreadModel != "posix") + Driver.Diag(diag::err_drv_argument_not_allowed_with) ---------------- This should be `ThreadModel == "single"`, since the distinction we care about is single-threaded vs no single-threaded. If in the future there are dozens of thread models besides "posix" this logic should still work. ================ Comment at: lib/Driver/ToolChains/WebAssembly.cpp:157 + parseThreadArgs(getDriver(), DriverArgs, Pthread, ThreadModel); + if (Pthread || ThreadModel == "posix") { + CC1Args.push_back("-target-feature"); ---------------- Same here. Should compare ThreadModel with "single". Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57874/new/ https://reviews.llvm.org/D57874 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits