delcypher added inline comments.
================ Comment at: clang/include/clang/Basic/CodeGenOptions.def:225-228 +ENUM_CODEGENOPT(SanitizeAddressDetectStackUseAfterReturnMode, + llvm::AsanDetectStackUseAfterReturnMode, 2, + llvm::AsanDetectStackUseAfterReturnMode::Runtime + ) ///< Set detection mode for stack-use-after-return. ---------------- vitalybuka wrote: > delcypher wrote: > > vitalybuka wrote: > > > vitalybuka wrote: > > > > Mode->Kind to be consistent with the file > > > I believe for consistency we need: > > > 1. Var name (SanitizeAddressDetectStackUseAfterReturn) no Mode/Kind suffix > > > 2. Enum type AsanDetectStackUseAfterReturnKind uses "kind" > > > 3. Actual flag includes no "kind" > > > -fsanitize-address-detect-stack-use-after-return= > > > > > > BTW. @delcypher -fsanitize-address-destructor-kind was not consistent > > > with the rest of file, but I am not sure it's worth of fixing now. > > > > > > > > @vitalybuka It's not too late to fix this because the > > `-fsanitize-address-destructor-kind=` flag hasn't been adopted yet AFAIK. > > > > Am I right in understanding that what I landed doesn't match points 1 and 3 > > above? If you can confirm that's what you'd like to be changed I can put up > > some patches to fix this. > Correct > I'd expect > > ``` > ENUM_CODEGENOPT(SanitizeAddressDtor, llvm::AsanDtorKind, 2, > llvm::AsanDtorKind::Global) > ``` > > and > > ``` > def sanitize_address_destructor_EQ > : Joined<["-"], "fsanitize-address-destructor=">, > MetaVarName<"<kind>">, > Flags<[CC1Option]>, > HelpText<"Set destructor type used in ASan instrumentation">, > Group<f_clang_Group>, > Values<"none,global">, > NormalizedValuesScope<"llvm::AsanDtorKind">, > NormalizedValues<["None", "Global"]>, > MarshallingInfoEnum<CodeGenOpts<"SanitizeAddressDtor">, "Global">; > ``` > > And one more thing, probably not very important, and matter of personal > preferences. > Maybe we don't need MetaVarName<"<kind>"> for both of these flags. > Without MetaVarName documentation is going to be > -fsanitize-address-destructor=<arg> > MetaVarName just lets use to replace <arg> with more common and specific > <file>, <dir>.. etc. > To me <kind> is almost generic as <arg>, so why not to stick to default. @vitalybuka I put up the following patches for review https://reviews.llvm.org/D101490 https://reviews.llvm.org/D101491 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101122/new/ https://reviews.llvm.org/D101122 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
