aheejin marked 4 inline comments as done.
aheejin added inline comments.

================
Comment at: lib/Driver/ToolChains/WebAssembly.cpp:29
+  Pthread =
+      DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+  ThreadModel =
----------------
tlively wrote:
> Shouldn't every use of `hasFlag` be `getLastArgValue` instead?
`hasFlag` is a convenient way to check everything with one function call. with 
`getLastArgValue` we have to call it twice (for example, for `-pthread` and for 
`-no-pthread`), and most importantly when both of them are given, calling 
`getLastArgValue` doesn't give you information on which one is the last. 
`hasFlag` takes care of that by taking the last one when both of them are 
given. So `-pthread -no-pthread` will return false, and `-no-pthread -pthread` 
will return true.

The reason I used `hasArg` below is just to check if the user gave it 
explicitly or not. And that's the reason I named variables `Pthread` and 
`HasPthread`.


================
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);
----------------
tlively wrote:
> It doesn't matter whether the user included the -pthread flag if they later 
> included the -no-pthread flag.
This `HasThreadModel` is only used with `HasPthread` below.
```
if (HasPthread && HasThreadModel && ThreadModel != "posix")
```

So this is just to check if this thread model value came from the default value 
or the user explicitly gave it.


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

Reply via email to