dblaikie added inline comments.

================
Comment at: clang/include/clang/Driver/Options.td:1089
+  LangOpts<"DoubleSquareBracketAttributes">,
+  Default<!strconcat(cpp11.KeyPath, "||", c2x.KeyPath)>,
   PosFlag<SetTrue, [], "Enable">, NegFlag<SetFalse, [], "Disable">,
----------------
jansvoboda11 wrote:
> Paul-C-Anagnostopoulos wrote:
> > jansvoboda11 wrote:
> > > Paul-C-Anagnostopoulos wrote:
> > > > jansvoboda11 wrote:
> > > > > Were you planning to refactor this too?
> > > > You can't use the paste operator (#) there because c2x is a defvar. 
> > > > Defvars and undefined identifiers are taken literally by paste, due to 
> > > > its frequent use in forming record and class names.
> > > > 
> > > > This behavior is noted in the Programmer's Reference, but I think I 
> > > > will add a more prominent note. I wonder if we should change the 
> > > > behavior so defvars are treated normally.
> > > I see. In that case, can you please undo the whitespace change?
> > Sure, but don't we try to avoid over-long lines in .td files, too?
> I think that would be nice, but I'm not sure what's LLVM's policy on making 
> whitespace-only changes (it makes `git blame` less useful). If we decide 
> that's fine, let's do it in a purely NFC commit.
Yeah - generally the attitude is to not wholesale reformat files generally, but 
if you're going to make a bunch of changes in a file that's out of conformance 
it can be reasonable/worthwhile to reformat that file before the work - so that 
the autoformatted changes aren't in conflict with the existing format of the 
file. (& yeah, format changes outside the semantic lines being changed in a 
patch is undesirable & should be done separately if it's being done at all)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101766

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to