[PATCH] Added LanguageStandard::LS_JavaScript to gate all JS-specific parsing.

Alexander Kornienko alexfh at google.com
Fri Nov 29 07:23:12 PST 2013



================
Comment at: lib/Format/Format.cpp:409
@@ +408,3 @@
+    if (Styles[i].Language == Styles[0].Language ||
+        Styles[i].Language == FormatStyle::LK_None) {
+      *Style = Styles[i];
----------------
Daniel Jasper wrote:
> I find it weird to have this second condition inside the loop as it can only ever be true for i == 1. But I can't really come up with a better way to write this.
Yeah, the whole loop could be replaced with a couple of if's, while we have just two languages ;)

================
Comment at: lib/Format/Format.cpp:412
@@ +411,3 @@
+      Style->Language = Styles[0].Language;
+      return Input.error();
+    }
----------------
Daniel Jasper wrote:
> This will never return an error, right? (because of the check in line 396).. If so, how about just return "OK"?
Done.

================
Comment at: lib/Format/Format.cpp:407
@@ +406,3 @@
+  }
+  for (unsigned i = Styles.size() - 1; i > 0; --i) {
+    if (Styles[i].Language == Styles[0].Language ||
----------------
Daniel Jasper wrote:
> Please document what this does.
Done.

================
Comment at: lib/Format/Format.cpp:399
@@ +398,3 @@
+
+  for (unsigned i = 1; i < Styles.size(); ++i) {
+    if (Styles[i].Language == FormatStyle::LK_None && i != 1)
----------------
Daniel Jasper wrote:
> Add:
> 
>   // Ensures that only the first configuration can skip languages and
>   // each language is configured at most once.
Done.

================
Comment at: lib/Format/Format.cpp:1295
@@ -1203,1 +1294,3 @@
 
+static const char *getLanguageName(FormatStyle::LanguageKind Language) {
+  switch (Language) {
----------------
Daniel Jasper wrote:
> Return StringRef.
I was thinking about this, but it seemed, that the difference is almost negligible in this case, so I've decided to go with the simpler version. But yes, if you prefer StringRef here, it totally makes sense to change it.

================
Comment at: lib/Format/Format.cpp:1645
@@ -1540,1 +1644,3 @@
 
+static void fillLanguageByFileName(StringRef FileName, FormatStyle *Style) {
+  if (FileName.endswith_lower(".c") || FileName.endswith_lower(".h") ||
----------------
Daniel Jasper wrote:
> What happens by default, i.e. if the file extensions matches none of these?
We don't fill the Language here, and use the language from the predefined style. But we can also set it to LK_None as a hint, that we probably don't want to format a file with unknown language. We'll probably have to default to C++, when formatting standard input, though.


http://llvm-reviews.chandlerc.com/D2242

BRANCH
  svn

ARCANIST PROJECT
  clang



More information about the cfe-commits mailing list