[PATCH] D104975: Implement P1949

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added a subscriber: rsmith.
aaron.ballman added inline comments.


================
Comment at: clang/lib/Lex/Lexer.cpp:1444
+    static const llvm::sys::UnicodeCharSet XIDContinueChars(XIDContinueRanges);
+    return C == '_' || XIDStartChars.contains(C) ||
+           XIDContinueChars.contains(C);
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Is looking at start chars correct? I was thinking this should only look at the continuation characters because `isAllowedInitiallyIDChar` handles the start of an identifier.
> Yes,  But Maybe it needs a comment.
> Per Unicode spec, XIDContinueChars is a superset of XIDStartChars, and in the code, we don't deduplicate the table to not make them too big.
> So we look at both tables, starting with start as it's the most likely to contain the character 
Ah, that explanation makes sense, thank you! I agree that a comment would be helpful there.


================
Comment at: clang/test/CXX/drs/dr2xx.cpp:600
 
-namespace dr248 { // dr248: yes c++11
-  // FIXME: Should this also apply to c++98 mode? This was a DR against C++98.
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > This means we're going to drop our support of this DR on https://clang.llvm.org/cxx_dr_status.html when that page gets regenerated. What should our status of that DR be with these changes?
> Good question!
> I will change it to `Superseded by P1949`. What do you think?
Because we're treating this as a DR, hopefully there's a DR number we could use instead of the paper number. @rsmith -- how should we handle this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975